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..343273a5ca7524 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 @@ -40,6 +40,7 @@ import com.google.devtools.build.lib.packages.TestSize; import com.google.devtools.build.lib.packages.TestTimeout; import com.google.devtools.build.lib.util.Pair; +import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.common.options.EnumConverter; import java.util.List; @@ -189,12 +190,27 @@ private TestParams createTestAction(int shards) { AnalysisEnvironment env = ruleContext.getAnalysisEnvironment(); ArtifactRoot root = config.getTestLogsDirectory(ruleContext.getRule().getRepository()); + // TODO(laszlocsomor), TODO(ulfjack): `isExecutedOnWindows` should use the execution platform, + // not the host platform. Once Bazel can tell apart these platforms, fix the right side of this + // initialization. + final boolean isExecutedOnWindows = OS.getCurrent() == OS.WINDOWS; + + final boolean isUsingTestWrapperInsteadOfTestSetupScript = + isExecutedOnWindows && + ruleContext + .getConfiguration() + .getFragment(TestConfiguration.class) + .isUsingWindowsNativeTestWrapper(); + NestedSetBuilder inputsBuilder = NestedSetBuilder.stableOrder(); inputsBuilder.addTransitive( NestedSetBuilder.create(Order.STABLE_ORDER, runfilesSupport.getRunfilesMiddleman())); - NestedSet testRuntime = PrerequisiteArtifacts.nestedSet( - ruleContext, "$test_runtime", Mode.HOST); - inputsBuilder.addTransitive(testRuntime); + + if (!isUsingTestWrapperInsteadOfTestSetupScript) { + NestedSet testRuntime = PrerequisiteArtifacts.nestedSet( + ruleContext, "$test_runtime", Mode.HOST); + inputsBuilder.addTransitive(testRuntime); + } TestTargetProperties testProperties = new TestTargetProperties( ruleContext, executionRequirements); @@ -202,8 +218,12 @@ private TestParams createTestAction(int shards) { final boolean collectCodeCoverage = config.isCodeCoverageEnabled() && instrumentedFiles != null; - Artifact testSetupScript = ruleContext.getHostPrerequisiteArtifact("$test_setup_script"); - inputsBuilder.add(testSetupScript); + Artifact testActionExecutable = + isUsingTestWrapperInsteadOfTestSetupScript + ? ruleContext.getHostPrerequisiteArtifact("$test_wrapper") + : ruleContext.getHostPrerequisiteArtifact("$test_setup_script"); + + inputsBuilder.add(testActionExecutable); Artifact testXmlGeneratorScript = ruleContext.getHostPrerequisiteArtifact("$xml_generator_script"); inputsBuilder.add(testXmlGeneratorScript); @@ -309,7 +329,8 @@ private TestParams createTestAction(int shards) { new TestRunnerAction( ruleContext.getActionOwner(), inputs, - testSetupScript, + testActionExecutable, + isUsingTestWrapperInsteadOfTestSetupScript, testXmlGeneratorScript, collectCoverageScript, testLog, 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..e848e09c190335 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 @@ -102,6 +102,11 @@ public class TestRunnerAction extends AbstractAction implements NotifyOnActionCa private final int runNumber; private final String workspaceName; + // True if using the Windows-native test wrapper (the "$test_wrapper" attribute). + // Should only be true when the execution OS is Windows and `--windows_native_test_wrapper` is + // enabled. + private final boolean useTestWrapperInsteadOfTestSetupSh; + // Mutable state related to test caching. Lazily initialized: null indicates unknown. private Boolean unconditionalExecution; @@ -136,6 +141,7 @@ private static ImmutableList list(Artifact... artifacts) { ActionOwner owner, Iterable inputs, Artifact testSetupScript, // Must be in inputs + boolean useTestWrapperInsteadOfTestSetupSh, Artifact testXmlGeneratorScript, // Must be in inputs @Nullable Artifact collectCoverageScript, // Must be in inputs, if not null Artifact testLog, @@ -159,6 +165,7 @@ private static ImmutableList list(Artifact... artifacts) { configuration.getActionEnvironment()); Preconditions.checkState((collectCoverageScript == null) == (coverageArtifact == null)); this.testSetupScript = testSetupScript; + this.useTestWrapperInsteadOfTestSetupSh = useTestWrapperInsteadOfTestSetupSh; this.testXmlGeneratorScript = testXmlGeneratorScript; this.collectCoverageScript = collectCoverageScript; this.configuration = Preconditions.checkNotNull(configuration); @@ -744,6 +751,10 @@ public Artifact getTestSetupScript() { return testSetupScript; } + public boolean isUsingTestWrapperInsteadOfTestSetupScript() { + return useTestWrapperInsteadOfTestSetupSh; + } + public Artifact getTestXmlGeneratorScript() { return testXmlGeneratorScript; } 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..735ab3d441c265 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 @@ -135,14 +135,17 @@ public abstract List exec( * * @param testAction The test action. * @return the command line as string list. - * @throws ExecException + * @throws ExecException */ 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): `executedOnWindows` 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 + final boolean executedOnWindows = (OS.getCurrent() == OS.WINDOWS); + final boolean useTestWrapper = testAction.isUsingTestWrapperInsteadOfTestSetupScript(); + + if (executedOnWindows && !useTestWrapper) { args.add(testAction.getShExecutable().getPathString()); args.add("-c"); args.add("$0 $*"); @@ -159,7 +162,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 +175,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 +185,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/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_base.py b/src/test/py/bazel/test_base.py index a5c6f70fbcbc35..8670f0da71b115 100644 --- a/src/test/py/bazel/test_base.py +++ b/src/test/py/bazel/test_base.py @@ -59,6 +59,9 @@ def setUp(self): os.path.join(test_tmpdir, 'tests_root')) self._temp = TestBase._CreateDirs(os.path.join(test_tmpdir, 'tmp')) self._test_cwd = tempfile.mkdtemp(dir=self._tests_root) + self._test_bazelrc = os.path.join(self._temp, 'test_bazelrc') + with open(self._test_bazelrc, 'wt') as f: + f.write('build --announce --jobs=8\n') os.chdir(self._test_cwd) def tearDown(self): @@ -236,7 +239,7 @@ def RunBazel(self, args, env_remove=None, env_add=None): """ return self.RunProgram([ self.Rlocation('io_bazel/src/bazel'), - '--bazelrc=/dev/null', + '--bazelrc=' + self._test_bazelrc, '--nomaster_bazelrc', ] + args, env_remove, env_add) 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..f995c0e8671aed --- /dev/null +++ b/src/test/py/bazel/test_wrapper_test.py @@ -0,0 +1,63 @@ +# 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/17, test_wrapper.exe cannot yet run tests. + self.AssertExitCode(exit_code, 3, stderr) + +if __name__ == '__main__': + unittest.main()