Try to deflake MacOSXFsEventsDiffAwarenessTest.
This test was using a short sleep to wait for fsevents to propagate and,
of course, sometimes we exhaust this timeout without seeing the events
(if the machine is very busy, for example). Or at least that's the
theory behind why this test has become flaky now.
Fix this by retrying the query a few times so that we wait for a very
short amount of time in the common case--but with the ability to tolerate
delays.
While doing this, significantly change how the test works to improve
readability and to cope with the fact that retrying the getDiff calls
requires changing the algorithm we use to check for success (given that
the arguments to this function have to be sequential views).
Tested: Manually ran this test on my Mac with --runs_per_test=100 and
with a very short sleep in each iteration (10ms instead of the 100ms
I'm adding here).
RELNOTES: None.
PiperOrigin-RevId: 294720397
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/MacOSXFsEventsDiffAwarenessTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/MacOSXFsEventsDiffAwarenessTest.java
index bd3c992..949a93b 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/MacOSXFsEventsDiffAwarenessTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/MacOSXFsEventsDiffAwarenessTest.java
@@ -14,8 +14,6 @@
package com.google.devtools.build.lib.skyframe;
-import static com.google.common.truth.Truth.assertThat;
-
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.skyframe.DiffAwareness.View;
@@ -30,7 +28,9 @@
import java.nio.file.Path;
import java.nio.file.SimpleFileVisitor;
import java.nio.file.attribute.BasicFileAttributes;
+import java.util.HashSet;
import java.util.Map;
+import java.util.Set;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@@ -89,37 +89,58 @@
scratchFile(path, "");
}
- private void assertDiff(View view1, View view2, Object... paths)
- throws IncompatibleViewException, BrokenDiffAwarenessException {
- ImmutableSet<PathFragment> modifiedSourceFiles =
- underTest.getDiff(view1, view2).modifiedSourceFiles();
- ImmutableSet<String> toStringSourceFiles = toString(modifiedSourceFiles);
- assertThat(toStringSourceFiles).containsExactly(paths);
- }
-
- private static ImmutableSet<String> toString(ImmutableSet<PathFragment> modifiedSourceFiles) {
- ImmutableSet.Builder<String> builder = ImmutableSet.builder();
- for (PathFragment path : modifiedSourceFiles) {
- if (!path.toString().isEmpty()) {
- builder.add(path.toString());
- }
+ /**
+ * Checks that the union of the diffs between the current view and each member of some consecutive
+ * sequence of views is the specific set of given files.
+ *
+ * @param view1 the view to compare to
+ * @param rawPaths the files to expect in the view
+ * @return the new view
+ */
+ private View assertDiff(View view1, String... rawPaths)
+ throws IncompatibleViewException, BrokenDiffAwarenessException, InterruptedException {
+ Set<PathFragment> pathsYetToBeSeen = new HashSet<>();
+ for (String path : rawPaths) {
+ pathsYetToBeSeen.add(PathFragment.create(path));
}
- return builder.build();
+
+ // fsevents may be delayed (especially under machine load), which means that we may not notice
+ // all file system changes in one go. Try enough times (multiple seconds) for the events to be
+ // delivered. Given that each time we call getCurrentView we may get a subset of the total
+ // events we expect, track the events we have already seen by subtracting them from the
+ // pathsYetToBeSeen set.
+ int attempts = 0;
+ for (; ; ) {
+ View view2 = underTest.getCurrentView(watchFsEnabledProvider);
+
+ ImmutableSet<PathFragment> modifiedSourceFiles =
+ underTest.getDiff(view1, view2).modifiedSourceFiles();
+ pathsYetToBeSeen.removeAll(modifiedSourceFiles);
+ if (pathsYetToBeSeen.isEmpty()) {
+ // Found all paths that we wanted to see as modified.
+ return view2;
+ }
+
+ if (attempts == 600) {
+ throw new AssertionError("Paths " + pathsYetToBeSeen + " not found as modified");
+ }
+ Thread.sleep(100);
+ attempts++;
+ view1 = view2; // getDiff requires views to be sequential if we want to get meaningful data.
+ }
}
@Test
public void testSimple() throws Exception {
View view1 = underTest.getCurrentView(watchFsEnabledProvider);
+
scratchFile("a/b/c");
scratchFile("b/c/d");
- Thread.sleep(200); // Wait until the events propagate
- View view2 = underTest.getCurrentView(watchFsEnabledProvider);
- assertDiff(view1, view2, "a", "a/b", "a/b/c", "b", "b/c", "b/c/d");
+ View view2 = assertDiff(view1, "a", "a/b", "a/b/c", "b", "b/c", "b/c/d");
+
rmdirs(watchedPath.resolve("a"));
rmdirs(watchedPath.resolve("b"));
- Thread.sleep(200); // Wait until the events propagate
- View view3 = underTest.getCurrentView(watchFsEnabledProvider);
- assertDiff(view2, view3, "a", "a/b", "a/b/c", "b", "b/c", "b/c/d");
+ assertDiff(view2, "a", "a/b", "a/b/c", "b", "b/c", "b/c/d");
}
/**
