Skip to content
This repository was archived by the owner on Oct 16, 2019. It is now read-only.

Update PHPUnit to 5.7 and 6.0. Update QA tools #66

Merged
merged 7 commits into from
Mar 8, 2017

Conversation

demiankatz
Copy link
Contributor

This pull request is an attempt to begin bringing this project in line with the newest QA standards of other ZF components. Work is based in part on zendframework/zend-http#115.

@Xerkus: this is as far as I was able to get today. I think this could be merged as-is, as it is useful progress, or we can leave this PR open and I can create a TODO checklist to get through all of the various steps necessary to bring the project up to date. Please let me know how you would like to proceed -- a series of incremental PR's, or one big one.

@Xerkus Xerkus changed the base branch from master to develop March 7, 2017 21:01
if ('@package_version@' !== $phpUnitVersion && version_compare($phpUnitVersion, '3.5.0', '<')) {
echo 'This version of PHPUnit (' . PHPUnit_Runner_Version::id() . ') is not supported in Zend Framework 2.x unit tests.' . PHP_EOL;
exit(1);
if (class_exists('PHPUnit_Runner_Version', true)) {
Copy link
Member

@Xerkus Xerkus Mar 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole check can safely be dropped

@@ -32,7 +33,7 @@ public function testSetRegion()
public function testSetInvalidRegionThrowsException()
{
$ec2 = new TestAmazonAbstract('TestAccessKey', 'TestSecretKey');
$this->setExpectedException(
$this->expectException(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setExpectedException was replaced by expectException() + expectExceptionMessage() in phpunit

@@ -32,7 +33,7 @@ public function testSetRegion()
public function testSetInvalidRegionThrowsException()
{
$ec2 = new TestAmazonAbstract('TestAccessKey', 'TestSecretKey');
$this->setExpectedException(
$this->expectException(
'ZendService\Amazon\Ec2\Exception\InvalidArgumentException',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strings with class names should be replaced with ::class and imported with use statements across all codebase for the benefit of static analysis tools.
Might be done as separate QA related PR along with coding styles fixes.

@Xerkus Xerkus self-assigned this Mar 7, 2017
@Xerkus Xerkus changed the title Update dependencies. [WIP] Update PHPUnit to 5.7 and 6.0. Update QA tools Mar 7, 2017
@demiankatz
Copy link
Contributor Author

@Xerkus, I believe I have taken care of all of your suggestions. If you would like me to rebase this so that all of the PHPUnit stuff is in a single commit, let me know and I'll be happy to take care of it tomorrow. Heading home for now! Thanks again for your help.

@Xerkus Xerkus force-pushed the dependencies branch 2 times, most recently from f8bfe7a to 0399f47 Compare March 8, 2017 05:17
@Xerkus
Copy link
Member

Xerkus commented Mar 8, 2017

I pushed few commits adding new travis config, composer.json scripts and bumping zend-http, which is causing problems at its lowest versions
php-cs-fixer is to be replaced in next PR for squizlabs/php_codesniffer, so cs-check and cs-fix scripts are essentially placeholders.
license-check script is a placeholder for malukenho/docheader tool to be introduced in following PRs

zend-http bump is causing merge conflicts, please rebase.

demiankatz and others added 6 commits March 8, 2017 08:29
- changed `PHPUnit_Framework_TestCase` to `PHPUnit\Framework\TestCase`
- changed `setExpectedException` to `expectException`
- added assertions to risky tests
- work around legacy PHPUnit_Runner_Version check
@demiankatz
Copy link
Contributor Author

@Xerkus, I have rebased this onto zendframework:develop. Please let me know if you would like me to do any further work on this, or if you prefer to take it from here. If you'd like to split up the remaining work, just let me know what steps I should work on.

@Xerkus Xerkus changed the title [WIP] Update PHPUnit to 5.7 and 6.0. Update QA tools Update PHPUnit to 5.7 and 6.0. Update QA tools Mar 8, 2017
@Xerkus
Copy link
Member

Xerkus commented Mar 8, 2017

@demiankatz thanks for your contribution.

Another QA changes remaining are getting rid of obsolete php-cs-fixer and introducing docheader. Would you like to do that too?

@Xerkus Xerkus merged commit 20e5991 into zendframework:develop Mar 8, 2017
@demiankatz
Copy link
Contributor Author

@Xerkus,

I started looking at docheader yesterday, but I got a bit confused when I realized that the tool seems to check the headers, but does not offer a way to automatically update them. Is it necessary to manually update all of the headers and only use docheader to remind us that changes are needed, or am I missing some tool to automatically make the comments uniform?

In any case, I'm happy to do the docheader and php-cs-fixer work; I will begin on php-cs-fixer right now. I'll wait to hear from you about docheader before proceeding on that point, since I don't want to waste time on manual effort if I don't have to (though I'm willing to do it if there's no better way).

thanks,
Demian

@froschdesign
Copy link
Member

@demiankatz

…but does not offer a way to automatically update them.

At the moment this is still a feature request.

@demiankatz
Copy link
Contributor Author

@froschdesign, good to know -- thanks for the clarification! That will definitely be a very valuable feature if implemented, but I'll deal with this manually in the meantime.

@Xerkus Xerkus added this to the 2.1.0 milestone Mar 9, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants