[Support] unflake TempFileCollisions test

[inglorion]

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).

[rsmith]

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

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.

[fedor.sergeev]

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

[inglorion]

Updated as suggested by @rsmith (thanks!)

[inglorion]

Does this look good now?

[fedor.sergeev]

Looks good!

llvm/unittests/Support/Path.cpp

if (T) {
       return true;
     } else {
       logAllUnhandledErrors(T.takeError(), errs(),
                             "Failed to create temporary file: ");
       return false;
     }
   };
 
-  // We should be able to create exactly 16 temporary files.
-  for (int i = 0; i < 16; ++i)
-    EXPECT_TRUE(TryCreateTempFile());
-  EXPECT_FALSE(TryCreateTempFile());
+  // Our single-character template allows for 16 unique names. Check that
+  // attempting to generate 17 results in TryCreateTempFile returning false
+  // (rather than something less desirable, such as infinite looping).
+  // This may happen before the 17th attempt due to the way names are generated.
+  bool Succeeded = true;
+  for (int i = 0; i < 17; ++i)
+    if (TryCreateTempFile()) Succeeded = false;
+  EXPECT_FALSE(Succeeded);
 
   for (fs::TempFile &T : TempFiles)
     cantFail(T.discard());
 }
 
 TEST_F(FileSystemTest, CreateDir) {
   ASSERT_NO_ERROR(fs::create_directory(Twine(TestDirectory) + "foo"));
   ASSERT_NO_ERROR(fs::create_directory(Twine(TestDirectory) + "foo"));