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.

Sign up for GitHub

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

Jump to bottom

flaky tests #120

Open
jmr opened this issue Mar 20, 2025 · 3 comments · May be fixed by #129
Open

flaky tests #120

jmr opened this issue Mar 20, 2025 · 3 comments · May be fixed by #129
Assignees
@rsned

Comments

@jmr
Copy link
Collaborator

jmr commented Mar 20, 2025

Some tests are flaky due to use of randomness. Here are the failure counts for 1000 runs:

     10 TestCellContainsPointConsistentWithS2CellIDFromPoint
      1 TestCellDistanceToEdge
     24 TestCellUnionNormalizePseudoRandom
      2 TestChordAngleBetweenPoints
      3 TestConvexHullQueryPointsInsideHull
     12 TestEdgeClippingClipToPaddedFace
      2 TestPaddedCellShrinkToFit
     24 TestTestingFractal
jmr added a commit to jmr/geo that referenced this issue Mar 20, 2025
@jmr
Deflake tests
a681026 
Some tests are flaky due to the random seed.  It's not clear whether the
problem is the code or the test.  These all need further investigation.

Just fix the seed for now so that the tests pass.

golang#120
@jmr jmr mentioned this issue Mar 20, 2025
Merged
@rsned rsned self-assigned this Mar 25, 2025
@rsned
Copy link
Collaborator

rsned commented Mar 25, 2025

Need to add a seed flag and plumb it through the tests.
If tests are still flaky after using a consistent seed throughout, will need to refactor the helpers that use the random generation to allow an optional rand.Source as needed.

@rsned rsned linked a pull request Mar 26, 2025 that will close this issue
Open
@rsned
Copy link
Collaborator

rsned commented Mar 27, 2025

If there are tests that are still flaky after using new seeded random in testing comes through, update the flagged flaky methods to create a specific random Source to pass into the calls to random things

e.g. r := rand.New(rand.NewSource(1))

@jmr
Copy link
Collaborator Author

jmr commented Mar 27, 2025

e.g. r := rand.New(rand.NewSource(1))

This should always be considered a temporary workaround. Tests should pass with all possible seeds.

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

@rsned rsned

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Changes to better support random seeds for flaky tests. rsned/geo
2 participants
@jmr @rsned