Skip to content

Conversation

@vincentstradiot
Copy link

Hi, here is a Pull Request that improves the test cases for StopWatch by:

  • restoring and replacing flaky asserts by a deterministic implementation
  • removing the need of running Thread.sleep in unit tests

Thank you for the review, and have a nice day.

Uncomment flaky assertions and replace with deterministic implementation

Signed-off-by: Vincent Stradiot <[email protected]>
Uncomment flaky assertions and replace with deterministic implementation

Signed-off-by: Vincent Stradiot <[email protected]>
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 15, 2025
Uncomment flaky assertions and replace with deterministic implementation

Signed-off-by: Vincent Stradiot <[email protected]>
Uncomment flaky assertions and replace with deterministic implementation

Signed-off-by: Vincent Stradiot <[email protected]>
@bclozel
Copy link
Member

bclozel commented Dec 17, 2025

Test hasn't been flaky over the last 6 months.

image

@bclozel bclozel closed this Dec 17, 2025
@bclozel bclozel added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Dec 17, 2025
@vincentstradiot
Copy link
Author

@bclozel They aren't flaky because all the asserts are commented out on the main branch, which is not ideal (see image below).
The purpose of the PR was to uncomment these, to actually assert meaningful things in the tests.

Up to you to decide. Let met know if you want me to reactivate/recreate this PR.

Screenshot 2025-12-17 115038

@bclozel
Copy link
Member

bclozel commented Dec 17, 2025

I don't think it's worth it in this current form. We could consider completely rewriting the class and its tests but it's definitely not our priority. Thanks for the suggestion.

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

Labels

status: declined A suggestion or change that we don't feel we should currently apply

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants