Skip to content

Commit aa45f5f

Browse files
coeuvrecopybara-github
authored andcommitted
Move integration tests for BwoB to a base class and add more tests there.
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
1 parent 26ec43d commit aa45f5f

File tree

5 files changed

+641
-330
lines changed

5 files changed

+641
-330
lines changed

src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ public Token getTokenIfNeedToExecute(
431431
MetadataHandler metadataHandler,
432432
ArtifactExpander artifactExpander,
433433
Map<String, String> remoteDefaultPlatformProperties,
434-
boolean isRemoteCacheEnabled)
434+
boolean loadCachedOutputMetadata)
435435
throws InterruptedException {
436436
// TODO(bazel-team): (2010) For RunfilesAction/SymlinkAction and similar actions that
437437
// produce only symlinks we should not check whether inputs are valid at all - all that matters
@@ -476,7 +476,7 @@ public Token getTokenIfNeedToExecute(
476476
if (entry != null
477477
&& !entry.isCorrupted()
478478
&& cacheConfig.storeOutputMetadata()
479-
&& isRemoteCacheEnabled) {
479+
&& loadCachedOutputMetadata) {
480480
// load remote metadata from action cache
481481
cachedOutputMetadata = loadCachedOutputMetadata(action, entry, metadataHandler);
482482
}
@@ -780,7 +780,7 @@ public Token getTokenUnconditionallyAfterFailureToRecordActionCacheHit(
780780
MetadataHandler metadataHandler,
781781
ArtifactExpander artifactExpander,
782782
Map<String, String> remoteDefaultPlatformProperties,
783-
boolean isRemoteCacheEnabled)
783+
boolean loadCachedOutputMetadata)
784784
throws InterruptedException {
785785
if (action != null) {
786786
removeCacheEntry(action);
@@ -793,7 +793,7 @@ public Token getTokenUnconditionallyAfterFailureToRecordActionCacheHit(
793793
metadataHandler,
794794
artifactExpander,
795795
remoteDefaultPlatformProperties,
796-
isRemoteCacheEnabled);
796+
loadCachedOutputMetadata);
797797
}
798798

799799
/** Returns an action key. It is always set to the first output exec path string. */

src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -603,15 +603,16 @@ Token checkActionCache(
603603
RemoteOptions remoteOptions;
604604
SortedMap<String, String> remoteDefaultProperties;
605605
EventHandler handler;
606-
boolean isRemoteCacheEnabled;
606+
boolean loadCachedOutputMetadata;
607607
try (SilentCloseable c = profiler.profile(ProfilerTask.ACTION_CHECK, action.describe())) {
608608
remoteOptions = this.options.getOptions(RemoteOptions.class);
609609
remoteDefaultProperties =
610610
remoteOptions != null
611611
? remoteOptions.getRemoteDefaultExecProperties()
612612
: ImmutableSortedMap.of();
613-
isRemoteCacheEnabled =
614-
(remoteOptions != null && remoteOptions.isRemoteCacheEnabled()) || outputService != null;
613+
loadCachedOutputMetadata =
614+
outputService
615+
!= null; // Only load cached output metadata if remote output service is available
615616
handler =
616617
options.getOptions(BuildRequestOptions.class).explanationPath != null ? reporter : null;
617618
token =
@@ -623,7 +624,7 @@ Token checkActionCache(
623624
metadataHandler,
624625
artifactExpander,
625626
remoteDefaultProperties,
626-
isRemoteCacheEnabled);
627+
loadCachedOutputMetadata);
627628
} catch (UserExecException e) {
628629
throw ActionExecutionException.fromExecException(e, action);
629630
}
@@ -681,7 +682,7 @@ public <T extends ActionContext> T getContext(Class<? extends T> type) {
681682
metadataHandler,
682683
artifactExpander,
683684
remoteDefaultProperties,
684-
isRemoteCacheEnabled);
685+
loadCachedOutputMetadata);
685686
}
686687
}
687688

src/test/java/com/google/devtools/build/lib/remote/BUILD

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,10 @@ java_test(
3737
size = "small",
3838
srcs = glob(
3939
["**/*.java"],
40-
exclude = NATIVE_SSL_TEST + ["BuildWithoutTheBytesIntegrationTest.java"],
40+
exclude = NATIVE_SSL_TEST + [
41+
"BuildWithoutTheBytesIntegrationTest.java",
42+
"BuildWithoutTheBytesIntegrationTestBase.java",
43+
],
4144
) + NATIVE_SSL_TEST_MAYBE,
4245
test_class = "com.google.devtools.build.lib.AllTests",
4346
deps = [
@@ -119,6 +122,24 @@ java_test(
119122
],
120123
)
121124

125+
java_library(
126+
name = "build_without_the_bytes_integration_test_base",
127+
srcs = [
128+
"BuildWithoutTheBytesIntegrationTestBase.java",
129+
],
130+
deps = [
131+
"//src/main/java/com/google/devtools/build/lib/actions",
132+
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
133+
"//src/main/java/com/google/devtools/build/lib/util:os",
134+
"//src/main/java/com/google/devtools/build/lib/util/io",
135+
"//src/main/java/com/google/devtools/build/lib/vfs",
136+
"//src/test/java/com/google/devtools/build/lib/buildtool/util",
137+
"//third_party:guava",
138+
"//third_party:junit4",
139+
"//third_party:truth",
140+
],
141+
)
142+
122143
java_test(
123144
name = "BuildWithoutTheBytesIntegrationTest",
124145
size = "large",
@@ -128,14 +149,14 @@ java_test(
128149
"//third_party/grpc-java:grpc-jar",
129150
],
130151
deps = [
152+
":build_without_the_bytes_integration_test_base",
131153
"//src/main/java/com/google/devtools/build/lib:runtime",
132154
"//src/main/java/com/google/devtools/build/lib/actions",
133155
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
156+
"//src/main/java/com/google/devtools/build/lib/dynamic",
134157
"//src/main/java/com/google/devtools/build/lib/remote",
135-
"//src/main/java/com/google/devtools/build/lib/sandbox",
158+
"//src/main/java/com/google/devtools/build/lib/standalone",
136159
"//src/main/java/com/google/devtools/build/lib/util:os",
137-
"//src/main/java/com/google/devtools/build/lib/vfs",
138-
"//src/test/java/com/google/devtools/build/lib/buildtool/util",
139160
"//src/test/java/com/google/devtools/build/lib/remote/util:integration_test_utils",
140161
"//third_party:guava",
141162
"//third_party:junit4",

0 commit comments

Comments
 (0)