test execution: use Windows-native test_wrapper

Use the not-yet-functional test_wrapper on Windows
when --windows_native_test_wrapper is enabled,
otherwise use the test-setup.sh script like other
platforms.

See bazelbuild#5508

Change-Id: I961b5a10b92758045aef4cc6a1fca125a82f7e5b

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,21 +190,40 @@ 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<Artifact> inputsBuilder = NestedSetBuilder.stableOrder();
     inputsBuilder.addTransitive(
         NestedSetBuilder.create(Order.STABLE_ORDER, runfilesSupport.getRunfilesMiddleman()));
-    NestedSet<Artifact> testRuntime = PrerequisiteArtifacts.nestedSet(
-        ruleContext, "$test_runtime", Mode.HOST);
-    inputsBuilder.addTransitive(testRuntime);
+
+    if (!isUsingTestWrapperInsteadOfTestSetupScript) {
+      NestedSet<Artifact> testRuntime = PrerequisiteArtifacts.nestedSet(
+          ruleContext, "$test_runtime", Mode.HOST);
+      inputsBuilder.addTransitive(testRuntime);
+    }
     TestTargetProperties testProperties = new TestTargetProperties(
         ruleContext, executionRequirements);
 
     // If the test rule does not provide InstrumentedFilesProvider, there's not much that we can do.
     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,

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<Artifact> list(Artifact... artifacts) {
       ActionOwner owner,
       Iterable<Artifact> 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<Artifact> 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;
   }

src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java

@@ -135,14 +135,17 @@ public abstract List<SpawnResult> exec(
    *
    * @param testAction The test action.
    * @return the command line as string list.
-   * @throws ExecException 
+   * @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:
-    // 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<String> getArgs(TestRunnerAction testAction) throws
 
     // Insert the command prefix specified by the "--run_under=<command-prefix>" 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<String> getArgs(TestRunnerAction testAction) throws
     return ImmutableList.copyOf(args);
   }
 
-  private static void addRunUnderArgs(TestRunnerAction testAction, List<String> args) {
+  private static void addRunUnderArgs(TestRunnerAction testAction, List<String> 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<String> 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);
     }

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",

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)
 

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()