This is an archive of the discontinued LLVM Phabricator instance.

[Support] unflake TempFileCollisions test
ClosedPublic

Authored by inglorion on Jan 4 2019, 2:37 PM.

Details

Summary

This test was added to verify that createUniqueEntity() does
not enter an infinite loop when all possible names are taken. However,
it also checked that all possible names are generated, which is flaky
(because the names are generated randomly). This change increases the
number of attempts we make to make flakes exceedingly
unlikely (3.88e-62).

Diff Detail

Repository
rL LLVM

Event Timeline

inglorion created this revision.Jan 4 2019, 2:37 PM
rsmith added a subscriber: rsmith.Feb 15 2019, 5:17 PM

This seems fine to me, but I think we can do better if we want to. How about: call TryCreateTempFile() N times, and check that it succeeds exactly 16 times. Pick N such that the probability of it failing to find all 16 possible file names (given that it has 128 retries for each call) is astronomically small. (Right now the test should fail somewhere around 0.03% of the time; if we made, say, 32 calls, the probability of a flake would be significantly lower than the probability of a flake due to spontaneous hardware failure (in order to flake, we must find only 15 of the 16 filenames with at least 17 * 128 + 15 guesses, the probability of which is 1 in 3.9x10^62).

llvm/unittests/Support/Path.cpp
710 ↗(On Diff #180328)

Is there a missing ! here? It looks like this test would pass if at least one call succeeds rather than if at least one call fails.

Herald added a project: Restricted Project. · View Herald TranscriptFeb 15 2019, 5:17 PM
fedor.sergeev requested changes to this revision.Feb 27 2019, 3:56 AM

Agreed with @rsmith, running 32 times and collecting the amount of successes == 16 should be a fine way to do this.

This revision now requires changes to proceed.Feb 27 2019, 3:56 AM
inglorion updated this revision to Diff 194421.Apr 9 2019, 5:18 PM
inglorion edited the summary of this revision. (Show Details)
inglorion added a reviewer: rsmith.
inglorion removed a subscriber: rsmith.

Updated as suggested by @rsmith (thanks!)

Does this look good now?

fedor.sergeev accepted this revision.Apr 21 2019, 7:22 AM

Looks good!

This revision is now accepted and ready to land.Apr 21 2019, 7:22 AM
This revision was automatically updated to reflect the committed changes.