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 2bd7daf1b9b62e..d75f64a5664140 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 @@ -165,6 +165,11 @@ public Object getDefault(AttributeMap rule) { .nonconfigurable("policy decision: should be consistent across configurations")) .add(attr("args", STRING_LIST)) // Input files for every test action + .add( + attr("$test_wrapper", LABEL) + .cfg(HostTransition.INSTANCE) + .singleArtifact() + .value(env.getToolsLabel("//tools/test:test_wrapper"))) .add( attr("$test_runtime", LABEL_LIST) .cfg(HostTransition.INSTANCE) 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 fffbe7a5ea1b86..7b3355323c160f 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 @@ -165,6 +165,12 @@ public static final RuleClass getTestBaseRule(String toolsRepository) { "policy decision: this should be consistent across configurations")) .add(attr("args", STRING_LIST)) // Input files for every test action + .add( + attr("$test_wrapper", LABEL) + .cfg(HostTransition.INSTANCE) + .singleArtifact() + .value( + labelCache.getUnchecked(toolsRepository + "//tools/test:test_wrapper"))) .add( attr("$test_runtime", LABEL_LIST) .cfg(HostTransition.INSTANCE) 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 40725da69d4a5a..7240ada764ae1a 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 @@ -188,12 +188,19 @@ private TestParams createTestAction(int shards) { BuildConfiguration config = ruleContext.getConfiguration(); AnalysisEnvironment env = ruleContext.getAnalysisEnvironment(); ArtifactRoot root = config.getTestLogsDirectory(ruleContext.getRule().getRepository()); + final boolean useWindowsNativeTestWrapper = + ruleContext + .getConfiguration() + .getFragment(TestConfiguration.class) + .getWindowsNativeTestWrapper(); NestedSetBuilder inputsBuilder = NestedSetBuilder.stableOrder(); inputsBuilder.addTransitive( NestedSetBuilder.create(Order.STABLE_ORDER, runfilesSupport.getRunfilesMiddleman())); - NestedSet testRuntime = PrerequisiteArtifacts.nestedSet( - ruleContext, "$test_runtime", Mode.HOST); + NestedSet testRuntime = + useWindowsNativeTestWrapper + ? PrerequisiteArtifacts.nestedSet(ruleContext, "$test_wrapper", Mode.HOST) + : PrerequisiteArtifacts.nestedSet(ruleContext, "$test_runtime", Mode.HOST); inputsBuilder.addTransitive(testRuntime); TestTargetProperties testProperties = new TestTargetProperties( ruleContext, executionRequirements); @@ -202,8 +209,11 @@ private TestParams createTestAction(int shards) { final boolean collectCodeCoverage = config.isCodeCoverageEnabled() && instrumentedFiles != null; - Artifact testSetupScript = ruleContext.getHostPrerequisiteArtifact("$test_setup_script"); - inputsBuilder.add(testSetupScript); + Artifact testWrapper = + useWindowsNativeTestWrapper + ? ruleContext.getHostPrerequisiteArtifact("$test_wrapper") + : ruleContext.getHostPrerequisiteArtifact("$test_setup_script"); + inputsBuilder.add(testWrapper); Artifact testXmlGeneratorScript = ruleContext.getHostPrerequisiteArtifact("$xml_generator_script"); inputsBuilder.add(testXmlGeneratorScript); @@ -309,7 +319,8 @@ private TestParams createTestAction(int shards) { new TestRunnerAction( ruleContext.getActionOwner(), inputs, - testSetupScript, + testWrapper, + useWindowsNativeTestWrapper, testXmlGeneratorScript, collectCoverageScript, testLog, diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java index 0d2f85b1cb24f9..44001c9e281295 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java @@ -216,6 +216,22 @@ public static class TestOptions extends FragmentOptions { ) public Label coverageReportGenerator; + @Option( + name = "windows_native_test_wrapper", + // Undocumented: this features is under development and not yet ready for production use. + // We define the flag only in order to be able to test the feature. + // Design: https://github.com/laszlocsomor/proposals/blob/win-test-runner/designs/2018-07-18-windows-native-test-runner.md + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + // Affects loading and analysis: this flag affects which target Bazel loads and creates test + // actions with on Windows. + effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, + defaultValue = "false", + help = + "On Windows: if true, uses the C++ test wrapper to run tests, otherwise uses " + + "tools/test/test-setup.sh as on other platforms. On other platforms: no-op." + ) + public boolean windowsNativeTestWrapper; + @Override public Map> getDefaultsLabels() { return ImmutableMap.>of( @@ -230,6 +246,7 @@ public FragmentOptions getHost() { // configuration. hostOptions.coverageSupport = this.coverageSupport; hostOptions.coverageReportGenerator = this.coverageReportGenerator; + hostOptions.windowsNativeTestWrapper = this.windowsNativeTestWrapper; return hostOptions; } } @@ -298,6 +315,10 @@ public Label getCoverageReportGenerator(){ return options.coverageReportGenerator; } + public boolean getWindowsNativeTestWrapper() { + return options.windowsNativeTestWrapper; + } + /** * @return number of times the given test should run. If the test doesn't match any of the * filters, runs it once. 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 5827488c09bb76..27ee7bf390b079 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 @@ -72,7 +72,7 @@ public class TestRunnerAction extends AbstractAction implements NotifyOnActionCa private static final String GUID = "cc41f9d0-47a6-11e7-8726-eb6ce83a8cc8"; - private final Artifact testSetupScript; + private final Artifact testWrapper; private final Artifact testXmlGeneratorScript; private final Artifact collectCoverageScript; private final BuildConfiguration configuration; @@ -102,6 +102,9 @@ public class TestRunnerAction extends AbstractAction implements NotifyOnActionCa private final int runNumber; private final String workspaceName; + // Takes the value of the `--windows_native_test_wrapper` flag. + private final boolean useWindowsNativeTestWrapper; + // Mutable state related to test caching. Lazily initialized: null indicates unknown. private Boolean unconditionalExecution; @@ -135,7 +138,8 @@ private static ImmutableList list(Artifact... artifacts) { TestRunnerAction( ActionOwner owner, Iterable inputs, - Artifact testSetupScript, // Must be in inputs + Artifact testWrapper, // Must be in inputs + boolean useWindowsNativeTestWrapper, Artifact testXmlGeneratorScript, // Must be in inputs @Nullable Artifact collectCoverageScript, // Must be in inputs, if not null Artifact testLog, @@ -158,7 +162,8 @@ private static ImmutableList list(Artifact... artifacts) { list(testLog, cacheStatus, coverageArtifact), configuration.getActionEnvironment()); Preconditions.checkState((collectCoverageScript == null) == (coverageArtifact == null)); - this.testSetupScript = testSetupScript; + this.testWrapper = testWrapper; + this.useWindowsNativeTestWrapper = useWindowsNativeTestWrapper; this.testXmlGeneratorScript = testXmlGeneratorScript; this.collectCoverageScript = collectCoverageScript; this.configuration = Preconditions.checkNotNull(configuration); @@ -740,8 +745,12 @@ public ImmutableSet getMandatoryOutputs() { return getOutputs(); } - public Artifact getTestSetupScript() { - return testSetupScript; + public Artifact getTestWrapper() { + return testWrapper; + } + + public boolean isUsingWindowsNativeTestWrapper() { + return useWindowsNativeTestWrapper; } public Artifact getTestXmlGeneratorScript() { 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 2088dabf5b0708..29bcb36ada05fe 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 @@ -139,17 +139,22 @@ public abstract List exec( */ public static ImmutableList getArgs(TestRunnerAction testAction) throws ExecException { List 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: - // testAction.getConfiguration().getTargetOS() == OS.WINDOWS - if (OS.getCurrent() == OS.WINDOWS) { + // TODO(ulfjack): Using OS.getCurrent() for `executedOnWindows` is incorrect in case of remote + // execution, where we should consider the target configuration, not the machine Bazel happens + // to run on. Change this to something like: + // executedOnWindows = testAction.getConfiguration().getTargetOS() == OS.WINDOWS + final boolean executedOnWindows = OS.getCurrent() == OS.WINDOWS; + + final boolean useWindowsNativeTestWrapper = testAction.isUsingWindowsNativeTestWrapper(); + + if (executedOnWindows && !useWindowsNativeTestWrapper) { args.add(testAction.getShExecutable().getPathString()); args.add("-c"); args.add("$0 $*"); } - Artifact testSetup = testAction.getTestSetupScript(); - args.add(testSetup.getExecPath().getCallablePathString()); + Artifact testWrapper = testAction.getTestWrapper(); + args.add(testWrapper.getExecPath().getCallablePathString()); if (testAction.isCoverageMode()) { args.add(testAction.getCollectCoverageScript().getExecPathString()); @@ -159,7 +164,7 @@ public static ImmutableList getArgs(TestRunnerAction testAction) throws // Insert the command prefix specified by the "--run_under=" option, if any. if (execSettings.getRunUnder() != null) { - addRunUnderArgs(testAction, args); + addRunUnderArgs(testAction, args, executedOnWindows); } // Execute the test using the alias in the runfiles tree, as mandated by the Test Encyclopedia. @@ -172,7 +177,8 @@ public static ImmutableList getArgs(TestRunnerAction testAction) throws return ImmutableList.copyOf(args); } - private static void addRunUnderArgs(TestRunnerAction testAction, List args) { + private static void addRunUnderArgs( + TestRunnerAction testAction, List args, boolean executedOnWindows) { TestTargetExecutionSettings execSettings = testAction.getExecutionSettings(); if (execSettings.getRunUnderExecutable() != null) { args.add(execSettings.getRunUnderExecutable().getRootRelativePath().getCallablePathString()); @@ -181,12 +187,14 @@ private static void addRunUnderArgs(TestRunnerAction testAction, List ar // --run_under commands that do not contain '/' are either shell built-ins or need to be // located on the PATH env, so we wrap them in a shell invocation. Note that we shell tokenize // the --run_under parameter and getCommand only returns the first such token. - boolean needsShell = !command.contains("/"); + boolean needsShell = + !command.contains("/") && (!executedOnWindows || !command.contains("\\")); if (needsShell) { - args.add(testAction.getShExecutable().getPathString()); + String shellExecutable = testAction.getShExecutable().getPathString(); + args.add(shellExecutable); args.add("-c"); args.add("\"$@\""); - args.add("/bin/sh"); // Sets $0. + args.add(shellExecutable); // Sets $0. } args.add(command); } diff --git a/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java b/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java index 5351a956c3a0f6..d9100d83fa2c51 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java @@ -72,7 +72,7 @@ private static class TestedStandaloneTestStrategy extends StandaloneTestStrategy public TestedStandaloneTestStrategy( ExecutionOptions executionOptions, BinTools binTools, Path tmpDirRoot) { - super(executionOptions, binTools, tmpDirRoot); + super(executionOptions, binTools, tmpDirRoot, false); } @Override diff --git a/src/test/py/bazel/BUILD b/src/test/py/bazel/BUILD index 9087fe0bd7acab..ca52a35653df91 100644 --- a/src/test/py/bazel/BUILD +++ b/src/test/py/bazel/BUILD @@ -138,6 +138,22 @@ py_test( }), ) +py_test( + name = "test_wrapper_test", + main = select({ + "//src/conditions:windows": "test_wrapper_test.py", + "//conditions:default": "empty_test.py", + }), + srcs = select({ + "//src/conditions:windows": ["test_wrapper_test.py"], + "//conditions:default": ["empty_test.py"], + }), + deps = select({ + "//src/conditions:windows": [":test_base"], + "//conditions:default": [], + }), +) + py_test( name = "query_test", size = "medium", diff --git a/src/test/py/bazel/test_wrapper_test.py b/src/test/py/bazel/test_wrapper_test.py new file mode 100644 index 00000000000000..92b8c44cb2f618 --- /dev/null +++ b/src/test/py/bazel/test_wrapper_test.py @@ -0,0 +1,64 @@ +# Copyright 2018 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os +import unittest +from src.test.py.bazel import test_base + + +class BazelCleanTest(test_base.TestBase): + + def testTestExecutionWithTestSetupShAndWithTestWrapperExe(self): + self.ScratchFile('WORKSPACE') + self.ScratchFile('foo/BUILD', [ + 'py_test(', + ' name = "x_test",', + ' srcs = ["x_test.py"],', + ')', + ]) + self.ScratchFile('foo/x_test.py', [ + 'from __future__ import print_function', + 'import unittest', + '', + 'class XTest(unittest.TestCase):', + ' def testFoo(self):', + ' print("lorem ipsum")', + '', + 'if __name__ == "__main__":', + ' unittest.main()', + ], executable = True) + + # Run test with test-setup.sh + exit_code, stdout, stderr = self.RunBazel([ + 'test', '//foo:x_test', '--test_output=streamed', '-t-', + '--nowindows_native_test_wrapper']) + self.AssertExitCode(exit_code, 0, stderr) + found = False + for line in stdout + stderr: + if 'lorem ipsum' in line: + found = True + if not found: + self.fail('FAIL: output:\n%s\n---' % '\n'.join(stderr + stdout)) + + # Run test with test_wrapper.exe + exit_code, _, stderr = self.RunBazel([ + 'test', '//foo:x_test', '--test_output=streamed', '-t-', + '--windows_native_test_wrapper']) + + # As of 2018-08-07, test_wrapper.exe cannot yet run tests. + self.AssertExitCode(exit_code, 3, stderr) + +if __name__ == '__main__': + unittest.main() + diff --git a/tools/BUILD b/tools/BUILD index 619257122f86d4..c69f44e8f435c5 100644 --- a/tools/BUILD +++ b/tools/BUILD @@ -66,7 +66,7 @@ filegroup( "//tools/objc:srcs", "//tools/python:embedded_tools", "//tools/runfiles:embedded_tools", - "//tools/test:srcs", + "//tools/test:embedded_tools", "//tools/test/LcovMerger/java/com/google/devtools/lcovmerger:embedded_tools", "//tools/osx/crosstool:srcs", "//tools/osx:srcs", diff --git a/tools/test/BUILD b/tools/test/BUILD index c6a83110330d89..4ed14b8f6e981b 100644 --- a/tools/test/BUILD +++ b/tools/test/BUILD @@ -33,7 +33,33 @@ filegroup( srcs = ["@bazel_tools//tools/test/LcovMerger/java/com/google/devtools/lcovmerger:Main"], ) +cc_binary( + name = "test_wrapper", + srcs = select({ + "@bazel_tools//src/conditions:windows": ["windows/test_wrapper.cc"], + + # Dummy source file, so this rule can be built on non-Windows platforms + # (when building all targets in this package). + "//conditions:default": ["windows/dummy.cc"], + }), + visibility = ["//visibility:private"], +) + filegroup( name = "srcs", srcs = glob(["**"]), ) + +filegroup( + name = "embedded_tools", + srcs = [ + "BUILD.tools", + "test-setup.sh", + "generate-xml.sh", + "collect_coverage.sh", + ] + glob(["LcovMerger/**"]) + select({ + "@bazel_tools//src/conditions:windows": ["test_wrapper"], + "//conditions:default": [], + }), + visibility = ["//tools:__pkg__"], +) diff --git a/tools/test/BUILD.tools b/tools/test/BUILD.tools new file mode 100644 index 00000000000000..58b21bfa08df4c --- /dev/null +++ b/tools/test/BUILD.tools @@ -0,0 +1,42 @@ +package(default_visibility = ["//visibility:public"]) + +# Members of this filegroup shouldn't have duplicate basenames, otherwise +# TestRunnerAction#getRuntimeArtifact() will get confused. +# Deprecated, do not use. +filegroup( + name = "runtime", + srcs = ["test-setup.sh"], +) + +filegroup( + name = "test_setup", + srcs = ["test-setup.sh"], +) + +filegroup( + name = "test_xml_generator", + srcs = ["generate-xml.sh"], +) + +filegroup( + name = "collect_coverage", + srcs = ["collect_coverage.sh"], +) + +filegroup( + name = "coverage_support", + srcs = ["collect_coverage.sh"], +) + +filegroup( + name = "coverage_report_generator", + srcs = ["@bazel_tools//tools/test/LcovMerger/java/com/google/devtools/lcovmerger:Main"], +) + +filegroup( + name = "test_wrapper", + srcs = select({ + "@bazel_tools//src/conditions:windows": ["test_wrapper.exe"], + "//conditions:default": ["test_wrapper"], + }), +) diff --git a/tools/test/windows/dummy.cc b/tools/test/windows/dummy.cc new file mode 100644 index 00000000000000..885062753a8fc9 --- /dev/null +++ b/tools/test/windows/dummy.cc @@ -0,0 +1,27 @@ +// Copyright 2018 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Empty implementation of the test wrapper. +// +// As of 2018-08-06, every platform uses //tools/test:test-setup.sh as the test +// wrapper, and we are working on introducing a C++ test wrapper for Windows. +// See https://github.com/laszlocsomor/proposals/blob/win-test-runner/designs/2018-07-18-windows-native-test-runner.md + +#include + +int main(int, char**) { + fprintf(stderr, + __FILE__ ": The C++ test wrapper is not used on this platform.\n"); + return 1; +} diff --git a/tools/test/windows/test_wrapper.cc b/tools/test/windows/test_wrapper.cc new file mode 100644 index 00000000000000..d413c18bcaf172 --- /dev/null +++ b/tools/test/windows/test_wrapper.cc @@ -0,0 +1,22 @@ +// Copyright 2018 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Test wrapper implementation for Windows. +// Design: https://github.com/laszlocsomor/proposals/blob/win-test-runner/designs/2018-07-18-windows-native-test-runner.md + +int main(int argc, char** argv) { + // TODO(laszlocsomor): Implement the functionality described in + // https://github.com/laszlocsomor/proposals/blob/win-test-runner/designs/2018-07-18-windows-native-test-runner.md + return 0; +}