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.

Sign up for GitHub

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

Jump to bottom

Update PHPUnit to version 7 #5351

Merged
merged 2 commits into from
Dec 13, 2019
Merged

Update PHPUnit to version 7 #5351

merged 2 commits into from
Dec 13, 2019

Conversation

johnsaigle
Copy link
Contributor

@johnsaigle johnsaigle commented Oct 21, 2019 •
edited
Loading

Brief summary of changes

Commands run

git checkout aces/major
make dev
composer update phpunit/phpunit phpunit/php-code-coverage phpunit/php-timer phpunit/php-token-stream phpunit/phpunit-mock-objects (PHPUnit dependencies were iteratively added to the above command until composer was certain that the requirements could be satisfied.

Required in order to install phpstan static analysis tool

Links to related tickets (GitHub, Redmine, ...)

@johnsaigle johnsaigle changed the base branch from minor to major October 21, 2019 21:34
@johnsaigle johnsaigle added Meta PR does something that organizes, upgrades, or manages the functionality of the codebase Testing PR contains test plan or automated test code (or config files for Travis) labels Oct 21, 2019
@johnsaigle johnsaigle changed the title 191021 update php unit Update PHPUnit to version 7 Oct 21, 2019
@johnsaigle johnsaigle changed the title Update PHPUnit to version 7 Update PHPUnit to version 7 (current: 6. latest: 8.4) Oct 21, 2019
@johnsaigle johnsaigle changed the title Update PHPUnit to version 7 (current: 6. latest: 8.4) Update PHPUnit to version 7 Oct 21, 2019
@johnsaigle johnsaigle closed this Oct 22, 2019
@johnsaigle johnsaigle reopened this Oct 22, 2019
@johnsaigle
Copy link
Contributor Author

This is passing Travis even though the build appears to be pending:

Screen Shot 2019-10-22 at 11 53 04

@johnsaigle johnsaigle requested a review from driusan October 22, 2019 15:53
maltheism
maltheism previously approved these changes Oct 22, 2019
View reviewed changes
Copy link
Member

@maltheism maltheism left a comment

Choose a reason for hiding this comment

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

The tests all worked

@kongtiaowang
Copy link
Contributor

kongtiaowang commented Oct 22, 2019 •
edited
Loading

@johnsaigle PHPUnit 8.4 is the latest version

@johnsaigle
Copy link
Contributor Author

@kongtiaowang Yes I know. I opened this PR as a way to be able to install phpstan, as I wrote in the description.

If your PR gets merged before this one, I'll close it.

@maltheism maltheism added the Passed manual tests PR has been successfully tested by at least one peer label Oct 22, 2019
@johnsaigle johnsaigle mentioned this pull request Oct 22, 2019
Closed
driusan
driusan requested changes Oct 25, 2019
View reviewed changes
Copy link
Collaborator

@driusan driusan left a comment

Choose a reason for hiding this comment

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

I don't understand the "composer update phpunit/phpunit phpunit/php-code-coverage phpunit/php-timer phpunit/php-token-stream phpunit/phpunit-mock-objects" part of the instructions.. why would devs need to manually update those after this PR?

(And more importantly, this has conflicts with composer.lock now)

@johnsaigle
Copy link
Contributor Author

@driusan I meant those are the commands I ran to create this PR. Devs don't have to run them.

@johnsaigle johnsaigle added the State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) label Oct 25, 2019
@kongtiaowang
Copy link
Contributor

PHPunit 8.4 is not stable in Travis, I will close my PR.

driusan
driusan previously approved these changes Oct 25, 2019
View reviewed changes
@johnsaigle johnsaigle removed the State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) label Oct 28, 2019
@johnsaigle johnsaigle dismissed stale reviews from driusan and maltheism via 671c56d October 28, 2019 17:35
@johnsaigle johnsaigle added the State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) label Nov 19, 2019
@johnsaigle johnsaigle removed the State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) label Nov 27, 2019
@johnsaigle johnsaigle changed the base branch from major to master December 3, 2019 17:38
driusan
driusan reviewed Dec 6, 2019
View reviewed changes
composer.json Outdated
@@ -15,8 +15,8 @@
"bjeavons/zxcvbn-php": "^0.4"
},
"require-dev" : {
"squizlabs/php_codesniffer" : "3.5.1",
"phpunit/phpunit" : "6.5.*",
"squizlabs/php_codesniffer" : "2.9.2",
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is PHPCS being downgraded?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably a rebase issue. I think this was issued before the PHPCS update. I'll change it.

@driusan
Copy link
Collaborator

driusan commented Dec 6, 2019

I'll merge this once it passes Travis

@driusan driusan merged commit fb98769 into aces:master Dec 13, 2019
@ridz1208 ridz1208 added this to the 23.0.0 milestone Dec 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@driusan driusan driusan approved these changes

@maltheism maltheism maltheism left review comments

Assignees
No one assigned
Labels
Meta PR does something that organizes, upgrades, or manages the functionality of the codebase Passed manual tests PR has been successfully tested by at least one peer Testing PR contains test plan or automated test code (or config files for Travis)
Projects
None yet
Milestone
23.0.0
Development

Successfully merging this pull request may close these issues.
5 participants
@johnsaigle @kongtiaowang @driusan @maltheism @ridz1208