Skip to content

Commit

Permalink
Move integration tests for BwoB to a base class and add more tests th…
Browse files Browse the repository at this point in the history
…ere.

Remove the test parameters since it adds a lot of test time but for most tests we don't need to test these combinations. If we care a specific case (e.g. toplevel, remote cache, etc.), we should have a test case.

PiperOrigin-RevId: 483705367
Change-Id: If4a166d2545bd7aea6fb63ec901a0ec889e88ca0
  • Loading branch information
coeuvre authored and Copybara-Service committed Oct 25, 2022
1 parent 26ec43d commit aa45f5f
Show file tree
Hide file tree
Showing 5 changed files with 641 additions and 330 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ public Token getTokenIfNeedToExecute(
MetadataHandler metadataHandler,
ArtifactExpander artifactExpander,
Map<String, String> remoteDefaultPlatformProperties,
boolean isRemoteCacheEnabled)
boolean loadCachedOutputMetadata)
throws InterruptedException {
// TODO(bazel-team): (2010) For RunfilesAction/SymlinkAction and similar actions that
// produce only symlinks we should not check whether inputs are valid at all - all that matters
Expand Down Expand Up @@ -476,7 +476,7 @@ public Token getTokenIfNeedToExecute(
if (entry != null
&& !entry.isCorrupted()
&& cacheConfig.storeOutputMetadata()
&& isRemoteCacheEnabled) {
&& loadCachedOutputMetadata) {
// load remote metadata from action cache
cachedOutputMetadata = loadCachedOutputMetadata(action, entry, metadataHandler);
}
Expand Down Expand Up @@ -780,7 +780,7 @@ public Token getTokenUnconditionallyAfterFailureToRecordActionCacheHit(
MetadataHandler metadataHandler,
ArtifactExpander artifactExpander,
Map<String, String> remoteDefaultPlatformProperties,
boolean isRemoteCacheEnabled)
boolean loadCachedOutputMetadata)
throws InterruptedException {
if (action != null) {
removeCacheEntry(action);
Expand All @@ -793,7 +793,7 @@ public Token getTokenUnconditionallyAfterFailureToRecordActionCacheHit(
metadataHandler,
artifactExpander,
remoteDefaultPlatformProperties,
isRemoteCacheEnabled);
loadCachedOutputMetadata);
}

/** Returns an action key. It is always set to the first output exec path string. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -603,15 +603,16 @@ Token checkActionCache(
RemoteOptions remoteOptions;
SortedMap<String, String> remoteDefaultProperties;
EventHandler handler;
boolean isRemoteCacheEnabled;
boolean loadCachedOutputMetadata;
try (SilentCloseable c = profiler.profile(ProfilerTask.ACTION_CHECK, action.describe())) {
remoteOptions = this.options.getOptions(RemoteOptions.class);
remoteDefaultProperties =
remoteOptions != null
? remoteOptions.getRemoteDefaultExecProperties()
: ImmutableSortedMap.of();
isRemoteCacheEnabled =
(remoteOptions != null && remoteOptions.isRemoteCacheEnabled()) || outputService != null;
loadCachedOutputMetadata =
outputService
!= null; // Only load cached output metadata if remote output service is available
handler =
options.getOptions(BuildRequestOptions.class).explanationPath != null ? reporter : null;
token =
Expand All @@ -623,7 +624,7 @@ Token checkActionCache(
metadataHandler,
artifactExpander,
remoteDefaultProperties,
isRemoteCacheEnabled);
loadCachedOutputMetadata);
} catch (UserExecException e) {
throw ActionExecutionException.fromExecException(e, action);
}
Expand Down Expand Up @@ -681,7 +682,7 @@ public <T extends ActionContext> T getContext(Class<? extends T> type) {
metadataHandler,
artifactExpander,
remoteDefaultProperties,
isRemoteCacheEnabled);
loadCachedOutputMetadata);
}
}

Expand Down
29 changes: 25 additions & 4 deletions src/test/java/com/google/devtools/build/lib/remote/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ java_test(
size = "small",
srcs = glob(
["**/*.java"],
exclude = NATIVE_SSL_TEST + ["BuildWithoutTheBytesIntegrationTest.java"],
exclude = NATIVE_SSL_TEST + [
"BuildWithoutTheBytesIntegrationTest.java",
"BuildWithoutTheBytesIntegrationTestBase.java",
],
) + NATIVE_SSL_TEST_MAYBE,
test_class = "com.google.devtools.build.lib.AllTests",
deps = [
Expand Down Expand Up @@ -119,6 +122,24 @@ java_test(
],
)

java_library(
name = "build_without_the_bytes_integration_test_base",
srcs = [
"BuildWithoutTheBytesIntegrationTestBase.java",
],
deps = [
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/util:os",
"//src/main/java/com/google/devtools/build/lib/util/io",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/test/java/com/google/devtools/build/lib/buildtool/util",
"//third_party:guava",
"//third_party:junit4",
"//third_party:truth",
],
)

java_test(
name = "BuildWithoutTheBytesIntegrationTest",
size = "large",
Expand All @@ -128,14 +149,14 @@ java_test(
"//third_party/grpc-java:grpc-jar",
],
deps = [
":build_without_the_bytes_integration_test_base",
"//src/main/java/com/google/devtools/build/lib:runtime",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/dynamic",
"//src/main/java/com/google/devtools/build/lib/remote",
"//src/main/java/com/google/devtools/build/lib/sandbox",
"//src/main/java/com/google/devtools/build/lib/standalone",
"//src/main/java/com/google/devtools/build/lib/util:os",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/test/java/com/google/devtools/build/lib/buildtool/util",
"//src/test/java/com/google/devtools/build/lib/remote/util:integration_test_utils",
"//third_party:guava",
"//third_party:junit4",
Expand Down
Loading

0 comments on commit aa45f5f

Please sign in to comment.