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

refactor(Controller_Test_Case) #2062

Merged
merged 2 commits into from
Mar 25, 2024
Merged

Conversation

lucatume
Copy link
Contributor

@lucatume lucatume commented Mar 18, 2024

🎫 Ticket

No ticket, born in the context of wp-browser update.

🗒️ Description

Rename the setUp method to setUpTestCase and annotate it with @before.
Rename the tearDown method to tearDownTestCase and annotate it with @after.
Not overriding the base test case setUp and tearDown methods allows the Controller_Test_Case to be used across different PHPUnit versions.
Newer versions would require the : void return type that older versions will not support.

Artifacts

There are no tests covering the Controller_Test_Case directly, but I've run Event Tickets and The Events Calendar tests relying on the Controller_Test_Case and did not run into issues.

✔️ Checklist

  • Changelog entry in the readme.txt file.
  • Code is covered by NEW wpunit or integration tests.
  • Code is covered by EXISTING wpunit or integration tests.
  • Are all the required tests passing?
  • Automated code review comments are addressed.
  • Have you added Artifacts?
  • Check the base branch for your PR.
  • Add your PR to the project board for the release.

Rename the `setUp` method to `setUpTestCase` and annotate it with `@before`.
Rename the `tearDown` method to `tearDownTestCase` and annotate it with `@after`.
Not overriding the base test case `setUp` and `tearDown` methods allows the `Controller_Test_Case` to be use across different PHPUnit versions.
Newer versions would require the `: void` return type that older versions will not support.
@lucatume lucatume self-assigned this Mar 18, 2024
@lucatume lucatume requested a review from Camwyn March 18, 2024 19:53
@lucatume lucatume marked this pull request as ready for review March 19, 2024 12:09
Base automatically changed from release/B24.cardinal to master March 20, 2024 17:12
Copy link
Member

@Camwyn Camwyn left a comment

Choose a reason for hiding this comment

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

I only see (current coed) impacts in ECP and ET.
This won't require changing anything and feels like an improvement.

Let's merge it and then update any existing release branches.

@lucatume lucatume merged commit 975f2ef into master Mar 25, 2024
10 of 11 checks passed
@bordoni bordoni deleted the test/controller-test-case-update branch March 26, 2024 01:15
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.

2 participants