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

Allow to work with PHPUnit 10 #585

Merged
merged 3 commits into from
Apr 5, 2023

Conversation

Jean85
Copy link
Contributor

@Jean85 Jean85 commented Jan 11, 2023

This draft of a PR was needed for me to work on PHPUnit 10 compat on another project, see facile-it/paraunit#172

It allows two packages to the next major version (5), with no other code modifications since there are no BC impacting Prophecy.

@stof
Copy link
Member

stof commented Jan 12, 2023

Can you rebase your branch to avoid the merge commit ?

Also, is this actually a draft PR ?

@Jean85 Jean85 force-pushed the allow-sebastian-comparator-5 branch from 71ea1bc to 8759ac2 Compare January 12, 2023 11:44
@Jean85
Copy link
Contributor Author

Jean85 commented Jan 12, 2023

Rebased.

I left it as draft since both majors of both packages are not released yet.

@stof
Copy link
Member

stof commented Jan 12, 2023

@sebastianbergmann is it safe for us to release this change now or do you expect to do more BC breaks in sebastian/comparator and sebastian/recursion-context before their stable 5.0 release ?

@sebastianbergmann
Copy link
Contributor

I consider any such changes highly unlikely.

@Jean85
Copy link
Contributor Author

Jean85 commented Jan 17, 2023

@sebastianbergmann @stof I've found a BC that's hitting this PR and it's going unnoticed due to the CI here not installing a dev dep: SebastianBergmann\Comparator\Factory if final and it's being extended in Prophecy\Comparator\Factory.

We could easily fix it with the decorator patter, but I don't know if it's to be considered a BC here (since no longer extending the class).

@Jean85 Jean85 force-pushed the allow-sebastian-comparator-5 branch from 6c03a2c to 236677c Compare January 17, 2023 22:05
@stof
Copy link
Member

stof commented Jan 18, 2023

@Jean85 you need to update the specs of that Factory

@Jean85
Copy link
Contributor Author

Jean85 commented Jan 23, 2023

@stof I didn't do it because, apart from the fact that I'm not skilled with PHPSpec, I'm not sure on how to proceed:

  • one test asserts that the class should extend the PHPUnit's one (should I change it to "not"? Should I expect it on the returned, decorated factory?)
  • The other one tests the behavior, which is changed...

@stof
Copy link
Member

stof commented Feb 1, 2023

I worked on preparing this work in #589

@Jean85 Jean85 force-pushed the allow-sebastian-comparator-5 branch from 236677c to db6fbf5 Compare February 3, 2023 12:21
@Jean85
Copy link
Contributor Author

Jean85 commented Feb 3, 2023

Rebased to include #589

@Jean85
Copy link
Contributor Author

Jean85 commented Feb 19, 2023

Closing and reopening to trigger a CI run, now that PHPUnit 10 is released.

@Jean85
Copy link
Contributor Author

Jean85 commented Feb 19, 2023

This would require phpspec/phpspec#1443 to test and work completely.

@Jean85
Copy link
Contributor Author

Jean85 commented Feb 24, 2023

Let's follow the suggestion from phpspec/phpspec#1443 (comment) and test on an optional CI job requiring master.

@ciaranmcnulty ciaranmcnulty merged commit aa07de8 into phpspec:master Apr 5, 2023
Jean85 added a commit to Jean85/prophecy-phpunit that referenced this pull request Apr 5, 2023
Jean85 added a commit to Jean85/prophecy-phpunit that referenced this pull request Apr 5, 2023
Jean85 added a commit to Jean85/prophecy-phpunit that referenced this pull request Apr 5, 2023
Jean85 added a commit to Jean85/prophecy-phpunit that referenced this pull request Apr 5, 2023
Jean85 added a commit to Jean85/prophecy-phpunit that referenced this pull request Apr 5, 2023
@Jean85 Jean85 deleted the allow-sebastian-comparator-5 branch April 26, 2023 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants