Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create tests for PSR-4 classes, refactor oldtests #730

Merged
merged 84 commits into from
May 19, 2022

Conversation

Art4
Copy link
Contributor

@Art4 Art4 commented Apr 25, 2022

The goal of this PR is to copy and revise as many tests as possible to test against the PSR-4 (namespaced) classes. The old tests for PSR-0 (without namespaces) will remain untouched if possible to ensure BC.

Update: After working my way through the oldtests folder, I decided to migrate these very old tests directly after all, to minimize future workload.

This PR relies on merging #722 and still needs some work.

This PR is a subtask of #731.

@Art4 Art4 changed the title Create tests for PSR-4 classes Create tests for PSR-4 classes, refactore oldtests May 5, 2022
@Art4 Art4 changed the title Create tests for PSR-4 classes, refactore oldtests Create tests for PSR-4 classes, refactor oldtests May 5, 2022
@Art4
Copy link
Contributor Author

Art4 commented May 5, 2022

For the migration of the tests from the tests/oldtests folder I created a script to combine all test classes as an array for the PHPUnit data providers. The script is not perfect and I still had to fix a lot of lines with the multiline edit in my IDE. But it saved me a lot of work.

<?php
// change the foldername you want to migrate
$foldername = 'first_item_title';
$combined = '';
$path = dirname(__FILE__) . DIRECTORY_SEPARATOR . 'tests' . DIRECTORY_SEPARATOR . 'oldtests' . DIRECTORY_SEPARATOR . $foldername;

$files = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($path, FilesystemIterator::SKIP_DOTS));

foreach($files as $file_path => $info)
{
	$contents = file_get_contents($file_path);
	$contents = trim($contents);
	$contents = str_replace('Atom_03', 'Atom_0.3', $contents);
	$contents = str_replace('Atom_10', 'Atom_1.0', $contents);
	$contents = str_replace('DC_10', 'DC_1.0', $contents);
	$contents = str_replace('DC_11', 'DC_1.1', $contents);
	$contents = str_replace('RSS_09', 'RSS_0.9', $contents);
	$contents = str_replace('RSS_10', 'RSS_1.0', $contents);
	$contents = str_replace('RSS_20', 'RSS_2.0', $contents);
	$contents = str_replace('_Netscape', '-Netscape', $contents);
	$contents = str_replace('_Userland', '-Userland', $contents);
	$contents = str_replace(' extends ', '\' => [ extends ', $contents);
	$contents = str_replace('class ', "			'", $contents);
	$contents = str_replace('		$this->data =', "<<<EOT\n", $contents);
	$contents = str_replace("<?php\n", '', $contents);
	$contents = str_replace("\n?>", '', $contents);

	$combined .= $contents . "\n\n";
}

$combined = "<?php\n\t\treturn [\n" . $combined . "\n\n\t\t];\n";

file_put_contents($path . DIRECTORY_SEPARATOR . 'combined.php', $combined);

@Art4 Art4 mentioned this pull request May 6, 2022
48 tasks
@Art4
Copy link
Contributor Author

Art4 commented May 6, 2022

This PR is ready for review, but still relies on merging of #722.

@Art4 Art4 marked this pull request as ready for review May 6, 2022 07:30
@Art4
Copy link
Contributor Author

Art4 commented May 18, 2022

I resolved all merge conflicts after merge of #722.

@mblaney
Copy link
Member

mblaney commented May 19, 2022

In general I would say the copyright year shouldn't change when it's the only modification to a file, but all good for now.

@mblaney mblaney merged commit 22b8eb0 into simplepie:master May 19, 2022
@Art4 Art4 deleted the create-tests-for-psr4-classes branch May 19, 2022 12:24
@Art4
Copy link
Contributor Author

Art4 commented May 19, 2022

Thank you. I agree with you in principle about the year but it was a follow up change from #727 to keep all headers consistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants