Remove hard-wired basenames for the test setup and the coverage collection script.
Accomplished by creating an explicit attribute for both of them on test actions.
RELNOTES: None.
PiperOrigin-RevId: 185132460
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
index 2292c9d..5ea8584 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
@@ -138,6 +138,14 @@
attr("$test_runtime", LABEL_LIST)
.cfg(HostTransition.INSTANCE)
.value(ImmutableList.of(env.getToolsLabel("//tools/test:runtime"))))
+ .add(attr("$test_setup_script", LABEL)
+ .cfg(HostTransition.INSTANCE)
+ .singleArtifact()
+ .value(env.getToolsLabel("//tools/test:test_setup")))
+ .add(attr("$collect_coverage_script", LABEL)
+ .cfg(HostTransition.INSTANCE)
+ .singleArtifact()
+ .value(env.getToolsLabel("//tools/test:collect_coverage")))
// Input files for test actions collecting code coverage
.add(
attr("$coverage_support", LABEL)
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
index 7a83ac5..6f7a1bf 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
@@ -190,6 +190,14 @@
.value(
ImmutableList.of(
labelCache.getUnchecked(toolsRepository + "//tools/test:runtime"))))
+ .add(attr("$test_setup_script", LABEL)
+ .cfg(HostTransition.INSTANCE)
+ .singleArtifact()
+ .value(labelCache.getUnchecked(toolsRepository + "//tools/test:test_setup")))
+ .add(attr("$collect_coverage_script", LABEL)
+ .cfg(HostTransition.INSTANCE)
+ .singleArtifact()
+ .value(labelCache.getUnchecked(toolsRepository + "//tools/test:collect_coverage")))
// Input files for test actions collecting code coverage
.add(
attr("$coverage_support", LABEL)
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java
index 37cfbbe..bbce349 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java
@@ -202,10 +202,16 @@
final boolean collectCodeCoverage = config.isCodeCoverageEnabled()
&& instrumentedFiles != null;
+ Artifact testSetupScript = ruleContext.getHostPrerequisiteArtifact("$test_setup_script");
+ inputsBuilder.add(testSetupScript);
+
+ Artifact collectCoverageScript = null;
TreeMap<String, String> extraTestEnv = new TreeMap<>();
TestTargetExecutionSettings executionSettings;
if (collectCodeCoverage) {
+ collectCoverageScript = ruleContext.getHostPrerequisiteArtifact("$collect_coverage_script");
+ inputsBuilder.add(collectCoverageScript);
inputsBuilder.addTransitive(instrumentedFiles.getCoverageSupportFiles());
// Add instrumented file manifest artifact to the list of inputs. This file will contain
// exec paths of all source files that should be included into the code coverage output.
@@ -301,7 +307,8 @@
}
env.registerAction(new TestRunnerAction(
- ruleContext.getActionOwner(), inputs, testRuntime,
+ ruleContext.getActionOwner(), inputs,
+ testSetupScript, collectCoverageScript,
testLog, cacheStatus,
coverageArtifact,
testProperties, extraTestEnv, executionSettings,
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java
index 8a7661f..0166545 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java
@@ -32,13 +32,11 @@
import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.NotifyOnActionCacheHit;
-import com.google.devtools.build.lib.actions.UserExecException;
import com.google.devtools.build.lib.analysis.RunfilesSupplierImpl;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.RunUnder;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.ImmutableIterable;
-import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.util.LoggingUtil;
import com.google.devtools.build.lib.vfs.FileSystem;
@@ -71,7 +69,8 @@
private static final String GUID = "cc41f9d0-47a6-11e7-8726-eb6ce83a8cc8";
- private final NestedSet<Artifact> runtime;
+ private final Artifact testSetupScript;
+ private final Artifact collectCoverageScript;
private final BuildConfiguration configuration;
private final TestConfiguration testConfiguration;
private final Artifact testLog;
@@ -133,7 +132,8 @@
TestRunnerAction(
ActionOwner owner,
Iterable<Artifact> inputs,
- NestedSet<Artifact> runtime, // Must be a subset of inputs
+ Artifact testSetupScript, // Must be in inputs
+ @Nullable Artifact collectCoverageScript, // Must be in inputs, if not null
Artifact testLog,
Artifact cacheStatus,
Artifact coverageArtifact,
@@ -151,7 +151,9 @@
// Note that this action only cares about the runfiles, not the mapping.
new RunfilesSupplierImpl(PathFragment.create("runfiles"), executionSettings.getRunfiles()),
list(testLog, cacheStatus, coverageArtifact));
- this.runtime = runtime;
+ Preconditions.checkState((collectCoverageScript == null) == (coverageArtifact == null));
+ this.testSetupScript = testSetupScript;
+ this.collectCoverageScript = collectCoverageScript;
this.configuration = Preconditions.checkNotNull(configuration);
this.testConfiguration =
Preconditions.checkNotNull(configuration.getFragment(TestConfiguration.class));
@@ -693,14 +695,12 @@
return getOutputs();
}
- public Artifact getRuntimeArtifact(String basename) throws ExecException {
- for (Artifact runtimeArtifact : runtime) {
- if (runtimeArtifact.getExecPath().getBaseName().equals(basename)) {
- return runtimeArtifact;
- }
- }
+ public Artifact getTestSetupScript() {
+ return testSetupScript;
+ }
- throw new UserExecException("'" + basename + "' not found in test runtime");
+ @Nullable public Artifact getCollectCoverageScript() {
+ return collectCoverageScript;
}
public PathFragment getShExecutable() {
diff --git a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java
index a6fbd41..b5748ee 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java
@@ -53,15 +53,12 @@
import java.util.Map;
/** Runs TestRunnerAction actions. */
+// TODO(bazel-team): add tests for this strategy.
@ExecutionStrategy(
contextType = TestActionContext.class,
name = {"standalone"}
)
public class StandaloneTestStrategy extends TestStrategy {
- // TODO(bazel-team) - add tests for this strategy.
- public static final String COLLECT_COVERAGE =
- "external/bazel_tools/tools/test/collect_coverage.sh";
-
private static final ImmutableMap<String, String> ENV_VARS =
ImmutableMap.<String, String>builder()
.put("TZ", "UTC")
@@ -121,7 +118,7 @@
Spawn spawn =
new SimpleSpawn(
action,
- getArgs(COLLECT_COVERAGE, action),
+ getArgs(action),
ImmutableMap.copyOf(env),
executionInfo.build(),
new RunfilesSupplierImpl(
diff --git a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
index 3b3e691..0e0dab0 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
@@ -156,14 +156,11 @@
* Generates a command line to run for the test action, taking into account coverage and {@code
* --run_under} settings.
*
- * @param coverageScript a script interjected between setup script and rest of command line to
- * collect coverage data. If this is an empty string, it is ignored.
* @param testAction The test action.
* @return the command line as string list.
* @throws ExecException
*/
- public static ImmutableList<String> getArgs(String coverageScript, TestRunnerAction testAction)
- throws ExecException {
+ public static ImmutableList<String> getArgs(TestRunnerAction testAction) throws ExecException {
List<String> args = Lists.newArrayList();
// TODO(ulfjack): This is incorrect for remote execution, where we need to consider the target
// configuration, not the machine Bazel happens to run on. Change this to something like:
@@ -174,11 +171,11 @@
args.add("$0 $*");
}
- Artifact testSetup = testAction.getRuntimeArtifact(TEST_SETUP_BASENAME);
+ Artifact testSetup = testAction.getTestSetupScript();
args.add(testSetup.getExecPath().getCallablePathString());
if (testAction.isCoverageMode()) {
- args.add(coverageScript);
+ args.add(testAction.getCollectCoverageScript().getExecPathString());
}
TestTargetExecutionSettings execSettings = testAction.getExecutionSettings();
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
index af7a541..5c50705 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
@@ -461,7 +461,7 @@
// It's unfortunate that this method requires the path to the coverage collection script.
// Fortunately, this is "blaze run" and not "blaze coverage", so it's okay unless someone
// calls "blaze run --collect_code_coverage".
- cmdLine = TestStrategy.getArgs("dummy-coverage-script", testAction);
+ cmdLine = TestStrategy.getArgs(testAction);
} catch (ExecException e) {
env.getReporter().handle(Event.error(e.getMessage()));
return BlazeCommandResult.exitCode(ExitCode.LOCAL_ENVIRONMENTAL_ERROR);
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerTestStrategy.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerTestStrategy.java
index 274c953..0fb173c 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerTestStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerTestStrategy.java
@@ -242,7 +242,7 @@
}
private List<String> getStartUpArgs(TestRunnerAction action) throws ExecException {
- List<String> args = getArgs(/*coverageScript=*/ "coverage-is-not-supported", action);
+ List<String> args = getArgs(action);
ImmutableList.Builder<String> startupArgs = ImmutableList.builder();
// Add test setup with no echo to prevent stdout corruption.
startupArgs.add(args.get(0)).add("--no_echo");
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java
index eacc3f6..2d68367 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java
@@ -125,7 +125,9 @@
"/bazel_tools_workspace/tools/genrule/BUILD", "exports_files(['genrule-setup.sh'])");
config.create("/bazel_tools_workspace/tools/test/BUILD",
- "filegroup(name = 'runtime', srcs = ['test-setup.sh'],)",
+ "filegroup(name = 'runtime', srcs = ['test-setup.sh'])",
+ "filegroup(name = 'test_setup', srcs = ['test-setup.sh'])",
+ "filegroup(name = 'collect_coverage', srcs = ['collect_coverage.sh'])",
"filegroup(name='coverage_support', srcs=['collect_coverage.sh','LcovMerger'])",
"filegroup(name = 'coverage_report_generator', srcs = ['coverage_report_generator.sh'])");
diff --git a/tools/test/BUILD b/tools/test/BUILD
index 5b9dd5e..bc70912 100644
--- a/tools/test/BUILD
+++ b/tools/test/BUILD
@@ -7,6 +7,16 @@
srcs = ["test-setup.sh"],
)
+filegroup(
+ name = "test_setup",
+ srcs = ["test-setup.sh"],
+)
+
+filegroup(
+ name = "collect_coverage",
+ srcs = ["collect_coverage.sh"],
+)
+
java_binary(
name = "LcovMerger",
srcs = glob(["LcovMerger/java/**/*.java"]),
