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

Upgrade to PHPUnit 11 #305

Closed
2 tasks done
emteknetnz opened this issue Sep 2, 2024 · 6 comments
Closed
2 tasks done

Upgrade to PHPUnit 11 #305

emteknetnz opened this issue Sep 2, 2024 · 6 comments

Comments

@emteknetnz
Copy link
Member

emteknetnz commented Sep 2, 2024

Acceptance criteria

Notes

  • Used https://github.com/emteknetnz/phpunit11-writer to automatically update tests
  • Also manually did updates to fix some tests
  • dataProviders are now static functions
  • annotations converted to attributes, most notably dataProvider
  • old code coverage annotations were removed rather than converted
  • addMethods() changed to onlyMethods()
  • ->will($this->returnValue(...)) manually changed to ->willReturn(...)
  • ->will($this->returnCallback(...)) manually changed to ->willReturnCallback(...)
  • ->withConsecutive(...) manually changed to a willReturnCallback(...) solution
  • abstract tests are no longer allowed (presumably only if in /tests/ dir, because SapphireTest + FunctionalTest are OK)
  • keys in dataProvider data must now match param names
  • expectNotice() / expectWarning() / expectStrict() / expectDeprecation() have all been removed

CMS 5 PRs

Recipe sink CI CMS 6

CMS 6 PRs (ensure CMS 5 PRs are merged, merged-up and rebased on before merging these)

Follow up PRs

@emteknetnz emteknetnz added this to the Silverstripe CMS 6.0 milestone Sep 2, 2024
@emteknetnz emteknetnz self-assigned this Sep 4, 2024
This was referenced Sep 4, 2024
@GuySartorelli
Copy link
Member

CMS 5 PR merged. Reassigning to Steve for CMS 6 PR prep

@GuySartorelli
Copy link
Member

I do note "Ensure any deprecation warnings in PHPUnit 9 have been resolved" has been ticked though it seems not enough CMS 5 PRs to actually achieve that?

@GuySartorelli
Copy link
Member

Changes required or questions asked in

Some PRs also need a rebase.

@GuySartorelli
Copy link
Member

PRs merged. I still don't think "Ensure any deprecation warnings in PHPUnit 9 have been resolved" is done? Reassigning to Steve to check that.

@GuySartorelli
Copy link
Member

New PRs merged.

Reassigning to Steve because "Ensure any deprecation warnings in PHPUnit 9 have been resolved" still doesn't seem to have been done. The branches that are still running PHPUnit 9 still seem to have deprecation warnings.

@emteknetnz
Copy link
Member Author

emteknetnz commented Sep 24, 2024

The AC of fixing phpunit 9 warnings meant fix it for CMS 6, not for CMS 5. I should have spelt that out clearer in the initial ACs.

Projects aren't going to be running our unit-tests so it does matters at all if we have a bunch of warnings about phpunit 10 in our CMS 5 builds, we've had them for probably well over a year at this point. If they still both you feel free to open a new card and link to the framework PR (I think) which has the custom error handler stuff which we'd need to backport to CMS 5 to resolve. Another option is just turn off phpunit deprecation warnings for CMS 5 builds because at this point they're irrelevant since we've just done the upgrade work in the CMS 6 branches.

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

No branches or pull requests

2 participants