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

Fixup a flaky test setup #1703

Merged
merged 1 commit into from
Nov 4, 2024
Merged

Fixup a flaky test setup #1703

merged 1 commit into from
Nov 4, 2024

Conversation

khk-globus
Copy link
Contributor

Ensure that infrastructure is in place before we let the tests poke at it. The flakiness is a very rare occurrence — I've only encountered it once, recently — but flaky == no good.

Type of change

  • Code maintenance/cleanup

Ensure that infrastructure is in place before we let the tests poke at it.  The
flakiness is a very rare occurrence -- I've only encountered it once, recently
-- but flaky == no good.
@khk-globus khk-globus added no-news-is-good-news This change does not require a news file quick-review Review of this should be quick and easy labels Nov 3, 2024
Copy link
Contributor

@LeiGlobus LeiGlobus left a comment

Choose a reason for hiding this comment

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

I guess some internals just takes a (fraction of a s) to startup? Easy to wait only in 20ms increments instead of trying to figure out what and how long.

@khk-globus
Copy link
Contributor Author

I was thinking less the "wait in increments" and more like "the tests assume this is exists, so we wait until this is in place." I had made the try* methods (following Qt's implementation of similar in their test framework) a couple of years ago, so they were easily at-hand to solve this. If there's a better way ... do let me know!

@khk-globus khk-globus merged commit ea61f89 into main Nov 4, 2024
17 checks passed
@khk-globus khk-globus deleted the fixup_a_flaky_test_setup branch November 4, 2024 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-news-is-good-news This change does not require a news file quick-review Review of this should be quick and easy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants