diff options
author | Ulf Adams <ulfjack@google.com> | 2016-10-14 14:20:31 +0000 |
---|---|---|
committer | Yun Peng <pcloudy@google.com> | 2016-10-14 20:25:05 +0000 |
commit | de14adebfcc0d66ff48aec9ace3a4d36343200f5 (patch) | |
tree | 21505128f8d0aacf9379e1ca378f0c9814f8e410 /src/test/java/com/google | |
parent | 8d0be8960e49e09c0cbcc8173ce538edfa3cb208 (diff) |
Make --watchfs a common command option.
Adding an options parameter to DiffAwareness#getCurrentView seems like the
simplest way to achieve that.
Alternatives considered:
1. Making the diff awareness modules stateful. However, I did not want to do so
as I've also been working on improving the module API to reduce state, or at
least to have a proper lifecycle management for any necessary state.
2. Making the watchFs flag a constructor parameter. However, that would also
invalidate any implementations that don't use the flag (of which we have
several).
3. Only passing in a single boolean flag instead of an options class provider;
however, this is a more principled, futureproof API, which allows other
modules / awareness implementations to use their own options.
RELNOTES: --watchfs is now a command option; the startup option of the same
name is deprecated. I.e., use bazel build --watchfs, not blaze --watchfs
build.
--
MOS_MIGRATED_REVID=136154395
Diffstat (limited to 'src/test/java/com/google')
3 files changed, 55 insertions, 21 deletions
diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/IncrementalLoadingTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/IncrementalLoadingTest.java index c137cd3dff..32e8b4b30a 100644 --- a/src/test/java/com/google/devtools/build/lib/pkgcache/IncrementalLoadingTest.java +++ b/src/test/java/com/google/devtools/build/lib/pkgcache/IncrementalLoadingTest.java @@ -57,6 +57,7 @@ import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionName; import com.google.devtools.common.options.Options; +import com.google.devtools.common.options.OptionsClassProvider; import java.io.FileNotFoundException; import java.io.IOException; import java.util.ArrayList; @@ -411,7 +412,7 @@ public class IncrementalLoadingTest { private View currentView; @Override - public View getCurrentView() { + public View getCurrentView(OptionsClassProvider options) { lastView = currentView; currentView = new View() {}; return currentView; diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/DiffAwarenessManagerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/DiffAwarenessManagerTest.java index b58abc1a54..e2aebba89e 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/DiffAwarenessManagerTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/DiffAwarenessManagerTest.java @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.vfs.ModifiedFileSet; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; +import com.google.devtools.common.options.OptionsClassProvider; import java.util.List; import java.util.Map; import javax.annotation.Nullable; @@ -42,7 +43,6 @@ import org.junit.runners.JUnit4; */ @RunWith(JUnit4.class) public class DiffAwarenessManagerTest { - private FileSystem fs; private Path root; protected EventCollectionApparatus events; @@ -67,7 +67,8 @@ public class DiffAwarenessManagerTest { assertEquals( "Expected EVERYTHING_MODIFIED since there are no factories", ModifiedFileSet.EVERYTHING_MODIFIED, - manager.getDiff(events.reporter(), pathEntry).getModifiedFileSet()); + manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY) + .getModifiedFileSet()); events.assertNoWarningsOrErrors(); } @@ -80,12 +81,12 @@ public class DiffAwarenessManagerTest { DiffAwarenessFactoryStub factory = new DiffAwarenessFactoryStub(); factory.inject(pathEntry, diffAwareness1); DiffAwarenessManager manager = new DiffAwarenessManager(ImmutableList.of(factory)); - manager.getDiff(events.reporter(), pathEntry); + manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY); assertFalse("diffAwareness1 shouldn't have been closed yet", diffAwareness1.closed()); manager.reset(); assertTrue("diffAwareness1 should have been closed by reset", diffAwareness1.closed()); factory.inject(pathEntry, diffAwareness2); - manager.getDiff(events.reporter(), pathEntry); + manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY); assertFalse("diffAwareness2 shouldn't have been closed yet", diffAwareness2.closed()); events.assertNoWarningsOrErrors(); } @@ -101,21 +102,26 @@ public class DiffAwarenessManagerTest { DiffAwarenessFactoryStub factory = new DiffAwarenessFactoryStub(); factory.inject(pathEntry, diffAwareness); DiffAwarenessManager manager = new DiffAwarenessManager(ImmutableList.of(factory)); - ProcessableModifiedFileSet firstProcessableDiff = manager.getDiff(events.reporter(), pathEntry); + ProcessableModifiedFileSet firstProcessableDiff = + manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY); assertEquals( "Expected EVERYTHING_MODIFIED on first call to getDiff", ModifiedFileSet.EVERYTHING_MODIFIED, firstProcessableDiff.getModifiedFileSet()); firstProcessableDiff.markProcessed(); - ProcessableModifiedFileSet processableDiff1 = manager.getDiff(events.reporter(), pathEntry); + ProcessableModifiedFileSet processableDiff1 = + manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY); assertEquals(diff1, processableDiff1.getModifiedFileSet()); - ProcessableModifiedFileSet processableDiff2 = manager.getDiff(events.reporter(), pathEntry); + ProcessableModifiedFileSet processableDiff2 = + manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY); assertEquals(ModifiedFileSet.union(diff1, diff2), processableDiff2.getModifiedFileSet()); processableDiff2.markProcessed(); - ProcessableModifiedFileSet processableDiff3 = manager.getDiff(events.reporter(), pathEntry); + ProcessableModifiedFileSet processableDiff3 = + manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY); assertEquals(diff3, processableDiff3.getModifiedFileSet()); events.assertNoWarningsOrErrors(); - ProcessableModifiedFileSet processableDiff4 = manager.getDiff(events.reporter(), pathEntry); + ProcessableModifiedFileSet processableDiff4 = + manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY); assertEquals(ModifiedFileSet.EVERYTHING_MODIFIED, processableDiff4.getModifiedFileSet()); events.assertContainsWarning("error"); } @@ -139,7 +145,8 @@ public class DiffAwarenessManagerTest { DiffAwarenessManager manager = new DiffAwarenessManager(ImmutableList.of(factory1, factory2, factory3)); - ProcessableModifiedFileSet processableDiff = manager.getDiff(events.reporter(), pathEntry); + ProcessableModifiedFileSet processableDiff = + manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY); events.assertNoWarningsOrErrors(); assertEquals( "Expected EVERYTHING_MODIFIED on first call to getDiff for diffAwareness1", @@ -147,7 +154,7 @@ public class DiffAwarenessManagerTest { processableDiff.getModifiedFileSet()); processableDiff.markProcessed(); - processableDiff = manager.getDiff(events.reporter(), pathEntry); + processableDiff = manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY); events.assertContainsEventWithFrequency("error in getCurrentView", 1); assertEquals( "Expected EVERYTHING_MODIFIED because of broken getCurrentView", @@ -156,18 +163,18 @@ public class DiffAwarenessManagerTest { processableDiff.markProcessed(); factory1.remove(pathEntry); - processableDiff = manager.getDiff(events.reporter(), pathEntry); + processableDiff = manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY); assertEquals( "Expected EVERYTHING_MODIFIED on first call to getDiff for diffAwareness2", ModifiedFileSet.EVERYTHING_MODIFIED, processableDiff.getModifiedFileSet()); processableDiff.markProcessed(); - processableDiff = manager.getDiff(events.reporter(), pathEntry); + processableDiff = manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY); assertEquals(diff2, processableDiff.getModifiedFileSet()); processableDiff.markProcessed(); - processableDiff = manager.getDiff(events.reporter(), pathEntry); + processableDiff = manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY); events.assertContainsEventWithFrequency("error in getDiff", 1); assertEquals( "Expected EVERYTHING_MODIFIED because of broken getDiff", @@ -176,14 +183,14 @@ public class DiffAwarenessManagerTest { processableDiff.markProcessed(); factory2.remove(pathEntry); - processableDiff = manager.getDiff(events.reporter(), pathEntry); + processableDiff = manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY); assertEquals( "Expected EVERYTHING_MODIFIED on first call to getDiff for diffAwareness3", ModifiedFileSet.EVERYTHING_MODIFIED, processableDiff.getModifiedFileSet()); processableDiff.markProcessed(); - processableDiff = manager.getDiff(events.reporter(), pathEntry); + processableDiff = manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY); assertEquals(diff3, processableDiff.getModifiedFileSet()); processableDiff.markProcessed(); } @@ -235,7 +242,7 @@ public class DiffAwarenessManagerTest { } @Override - public View getCurrentView() throws BrokenDiffAwarenessException { + public View getCurrentView(OptionsClassProvider options) throws BrokenDiffAwarenessException { if (curSequenceNum == brokenViewNum) { throw new BrokenDiffAwarenessException("error in getCurrentView"); } 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 490c002fdd..a76184c989 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 @@ -18,7 +18,10 @@ import static com.google.common.truth.Truth.assertThat; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.skyframe.DiffAwareness.View; +import com.google.devtools.build.lib.skyframe.LocalDiffAwareness.Options; import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.common.options.OptionsBase; +import com.google.devtools.common.options.OptionsClassProvider; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.FileVisitResult; @@ -57,11 +60,15 @@ public class MacOSXFsEventsDiffAwarenessTest { private MacOSXFsEventsDiffAwareness underTest; private Path watchedPath; + private OptionsClassProvider watchFsEnabledProvider; @Before public void setUp() throws Exception { watchedPath = com.google.common.io.Files.createTempDir().getCanonicalFile().toPath(); underTest = new MacOSXFsEventsDiffAwareness(watchedPath.toString()); + LocalDiffAwareness.Options localDiffOptions = new LocalDiffAwareness.Options(); + localDiffOptions.watchFS = true; + watchFsEnabledProvider = new LocalDiffAwarenessOptionsProvider(localDiffOptions); } @After @@ -100,16 +107,35 @@ public class MacOSXFsEventsDiffAwarenessTest { @Test public void testSimple() throws Exception { - View view1 = underTest.getCurrentView(); + 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(); + View view2 = underTest.getCurrentView(watchFsEnabledProvider); assertDiff(view1, view2, "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(); + View view3 = underTest.getCurrentView(watchFsEnabledProvider); assertDiff(view2, view3, "a", "a/b", "a/b/c", "b", "b/c", "b/c/d"); } + + /** + * Only returns a fixed options class for {@link LocalDiffAwareness.Options}. + */ + private static final class LocalDiffAwarenessOptionsProvider implements OptionsClassProvider { + private final Options localDiffOptions; + + private LocalDiffAwarenessOptionsProvider(Options localDiffOptions) { + this.localDiffOptions = localDiffOptions; + } + + @Override + public <O extends OptionsBase> O getOptions(Class<O> optionsClass) { + if (optionsClass.equals(LocalDiffAwareness.Options.class)) { + return optionsClass.cast(localDiffOptions); + } + return null; + } + } } |