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

Fix unstable time based PHPUnit tests #8488

Closed
1 task done
benbowler opened this issue Apr 5, 2024 · 3 comments
Closed
1 task done

Fix unstable time based PHPUnit tests #8488

benbowler opened this issue Apr 5, 2024 · 3 comments
Labels
P1 Medium priority PHP QA: Eng Requires specialized QA by an engineer Squad 1 (Team S) Issues for Squad 1 Type: Infrastructure Engineering infrastructure & tooling

Comments

@benbowler
Copy link
Collaborator

benbowler commented Apr 5, 2024

Bug Description

Despite #6758 and this comment a couple of time based tests were missed and remain in the codebase causing instability:

https://github.com/google/site-kit-wp/blob/4f1d3fb446329ff3022fe3085633763cdd02cc94/tests/phpunit/integration/Core/Prompts/REST_Prompts_ControllerTest.php#L79-L91

https://github.com/google/site-kit-wp/blob/4f1d3fb446329ff3022fe3085633763cdd02cc94/tests/phpunit/integration/Core/Prompts/REST_Prompts_ControllerTest.php#L134-L142


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • Ensure that the tests listed above use the assertEqualsWithDelta assertion for time related keys to ensure better test stability when run on CI.

Implementation Brief

  • Split out the use of assertEqualSets in the tests above to separate uses of assertEquals and assertEqualsWithDelta.

Test Coverage

  • Ensure tests still cover the original cases and pass.

QA Brief

  • QA:Eng to confirm tests cover existing use cases.

Changelog entry

  • Fix unstable time based PHPUnit tests.
@benbowler benbowler self-assigned this Apr 5, 2024
@benbowler benbowler added P1 Medium priority QA: Eng Requires specialized QA by an engineer Type: Infrastructure Engineering infrastructure & tooling PHP labels Apr 5, 2024
@benbowler
Copy link
Collaborator Author

I created a candidate PR as this is such a small change.

@benbowler benbowler removed their assignment Apr 5, 2024
@bethanylang bethanylang added the Squad 1 (Team S) Issues for Squad 1 label Apr 5, 2024
@ivonac4 ivonac4 added the Next Up Issues to prioritize for definition label Apr 5, 2024
@eugene-manuilov eugene-manuilov self-assigned this Apr 6, 2024
@eugene-manuilov
Copy link
Collaborator

IB ✔️

@bethanylang bethanylang removed the Next Up Issues to prioritize for definition label Apr 8, 2024
@benbowler benbowler removed their assignment Apr 8, 2024
@mohitwp mohitwp removed their assignment Apr 10, 2024
@zutigrm zutigrm assigned zutigrm and unassigned zutigrm Apr 10, 2024
@zutigrm
Copy link
Collaborator

zutigrm commented Apr 15, 2024

QA:eng Verified

  • Did not find other occurrence in the code that uses assertEqualSets with time()
  • No tests failed, run it few times

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 Medium priority PHP QA: Eng Requires specialized QA by an engineer Squad 1 (Team S) Issues for Squad 1 Type: Infrastructure Engineering infrastructure & tooling
Projects
None yet
Development

No branches or pull requests

7 participants