Skip to content

Commit 0c8bc9d

Browse files
jincopybara-github
authored andcommitted
skyfocus: create SkyfocusIntegrationTest as a Java integration test.
focus_test takes a while to run now, and running a debugger over it isn't as straightforward as a Java integration test. It'll make testing subsequent CLs easier. This CL starts by moving the smoke test to the Java version. The Skyfocus preparation and execution steps are also moved into `CommandEnvironment` and `BuildTool` respectively, so we don't need to change `BlazeRuntimeWrapper` which mimics `BlazeCommandDispatcher`. PiperOrigin-RevId: 622759922 Change-Id: I404f30dd957bfea521ddb2af980531bfd641f841
1 parent 3f9d80c commit 0c8bc9d

File tree

6 files changed

+74
-38
lines changed

6 files changed

+74
-38
lines changed

src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,10 @@ public void buildTargets(BuildRequest request, BuildResult result, TargetValidat
282282
// The workspace status actions will not run with certain flags, or if an error occurs early
283283
// in the build. Ensure that build info is posted on every build.
284284
env.ensureBuildInfoPosted();
285+
286+
// Skyfocus runs at the end of the build.
287+
env.getSkyframeExecutor()
288+
.runSkyfocus(env.getReporter(), env.getBlazeWorkspace().getPersistentActionCache());
285289
}
286290
}
287291
}
@@ -559,11 +563,6 @@ public BuildResult processRequest(
559563
InterruptedFailureDetails.detailedExitCode("post build callback interrupted");
560564
}
561565
}
562-
563-
if (detailedExitCode.isSuccess()) {
564-
env.getSkyframeExecutor()
565-
.runSkyfocus(env.getReporter(), env.getBlazeWorkspace().getPersistentActionCache());
566-
}
567566
} catch (BuildFailedException e) {
568567
if (!e.isErrorAlreadyShown()) {
569568
// The actual error has not already been reported by the Builder.

src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@
5252
import com.google.devtools.build.lib.server.FailureDetails;
5353
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
5454
import com.google.devtools.build.lib.skyframe.RepositoryMappingValue.RepositoryMappingResolutionException;
55-
import com.google.devtools.build.lib.skyframe.SkyfocusOptions;
5655
import com.google.devtools.build.lib.util.AbruptExitException;
5756
import com.google.devtools.build.lib.util.AnsiStrippingOutputStream;
5857
import com.google.devtools.build.lib.util.DebugLoggerConfigurator;
@@ -563,18 +562,6 @@ private BlazeCommandResult execExclusively(
563562
return result;
564563
}
565564

566-
if (env.commandActuallyBuilds()) {
567-
// Need to determine if Skyfocus will run for this command. If so, the evaluator
568-
// will need to be configured to remember additional state (e.g. root keys) that it
569-
// otherwise doesn't need to for a non-Skyfocus build. Alternately, it might reset
570-
// the evaluator, which is why this runs before injecting precomputed values below.
571-
env.getSkyframeExecutor()
572-
.prepareForSkyfocus(
573-
env.getOptions().getOptions(SkyfocusOptions.class),
574-
env.getReporter(),
575-
env.getRuntime().getProductName());
576-
}
577-
578565
for (BlazeModule module : runtime.getBlazeModules()) {
579566
try (SilentCloseable closeable =
580567
Profiler.instance().profile(module + ".injectExtraPrecomputedValues")) {

src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import com.google.devtools.build.lib.server.FailureDetails;
4949
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
5050
import com.google.devtools.build.lib.skyframe.BuildResultListener;
51+
import com.google.devtools.build.lib.skyframe.SkyfocusOptions;
5152
import com.google.devtools.build.lib.skyframe.SkyframeBuildView;
5253
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
5354
import com.google.devtools.build.lib.skyframe.WorkspaceInfoFromDiff;
@@ -842,6 +843,15 @@ public void beforeCommand(InvocationPolicy invocationPolicy) throws AbruptExitEx
842843

843844
// Modules that are subscribed to CommandStartEvent may create pending exceptions.
844845
throwPendingException();
846+
847+
if (commandActuallyBuilds()) {
848+
// Need to determine if Skyfocus will run for this command. If so, the evaluator
849+
// will need to be configured to remember additional state (e.g. root keys) that it
850+
// otherwise doesn't need to for a non-Skyfocus build. Alternately, it might reset
851+
// the evaluator, which is why this runs before injecting precomputed values below.
852+
skyframeExecutor.prepareForSkyfocus(
853+
options.getOptions(SkyfocusOptions.class), reporter, runtime.getProductName());
854+
}
845855
}
846856

847857
/** Returns the name of the file system we are writing output to. */

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -744,6 +744,15 @@ java_test(
744744
],
745745
)
746746

747+
java_test(
748+
name = "SkyfocusIntegrationTest",
749+
srcs = ["SkyfocusIntegrationTest.java"],
750+
deps = [
751+
"//src/test/java/com/google/devtools/build/lib/buildtool/util",
752+
"//third_party:junit4",
753+
],
754+
)
755+
747756
java_test(
748757
name = "ProjectResolutionTest",
749758
srcs = ["ProjectResolutionTest.java"],
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// Copyright 2024 The Bazel Authors. All rights reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
package com.google.devtools.build.lib.buildtool;
15+
16+
import com.google.devtools.build.lib.buildtool.util.BuildIntegrationTestCase;
17+
import org.junit.Test;
18+
import org.junit.runner.RunWith;
19+
import org.junit.runners.JUnit4;
20+
21+
/** Skyfocus integration tests */
22+
@RunWith(JUnit4.class)
23+
public final class SkyfocusIntegrationTest extends BuildIntegrationTestCase {
24+
25+
@Override
26+
protected void setupOptions() throws Exception {
27+
super.setupOptions();
28+
addOptions("--experimental_enable_skyfocus");
29+
}
30+
31+
@Test
32+
public void workingSet_canBeUsedWithBuildCommand() throws Exception {
33+
addOptions("--experimental_working_set=hello/x.txt", "--experimental_skyfocus_dump_keys=count");
34+
write("hello/x.txt", "x");
35+
write(
36+
"hello/BUILD",
37+
"""
38+
genrule(
39+
name = "target",
40+
srcs = ["x.txt"],
41+
outs = ["out"],
42+
cmd = "cat $< > $@",
43+
)
44+
""");
45+
46+
buildTarget("//hello/...");
47+
assertContainsEvent("Updated working set successfully.");
48+
assertContainsEvent("Focusing on");
49+
assertContainsEvent("Node count:");
50+
}
51+
}

src/test/shell/integration/focus_test.sh

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -70,26 +70,6 @@ function set_up() {
7070
export DONT_SANITY_CHECK_SERIALIZATION=1
7171
}
7272

73-
function test_working_set_can_be_used_with_build_command() {
74-
local -r pkg=${FUNCNAME[0]}
75-
mkdir ${pkg}|| fail "cannot mkdir ${pkg}"
76-
mkdir -p ${pkg}
77-
echo "input" > ${pkg}/in.txt
78-
cat > ${pkg}/BUILD <<EOF
79-
genrule(
80-
name = "g",
81-
srcs = ["in.txt"],
82-
outs = ["out.txt"],
83-
cmd = "cp \$< \$@",
84-
)
85-
EOF
86-
87-
bazel build //${pkg}:g \
88-
--experimental_working_set=${pkg}/in.txt >$TEST_log 2>&1 \
89-
|| fail "unexpected failure"
90-
expect_log "Focusing on"
91-
}
92-
9373
function test_correctly_rebuilds_with_working_set_containing_files() {
9474
local -r pkg=${FUNCNAME[0]}
9575
mkdir ${pkg}|| fail "cannot mkdir ${pkg}"

0 commit comments

Comments
 (0)