Windows,test wrapper: pass command line arguments
The Windows native test wrapper now passes command
line arguments to the test binary.
See https://github.com/bazelbuild/bazel/issues/5508
Change-Id: I71e1c3424978542f5f36061a933dcfaab76b9b05
Closes #6278.
Change-Id: I71e1c3424978542f5f36061a933dcfaab76b9b05
PiperOrigin-RevId: 215359150
diff --git a/src/test/py/bazel/test_wrapper_test.py b/src/test/py/bazel/test_wrapper_test.py
index ec8f190..6058b1a 100644
--- a/src/test/py/bazel/test_wrapper_test.py
+++ b/src/test/py/bazel/test_wrapper_test.py
@@ -33,6 +33,25 @@
def _CreateMockWorkspace(self):
self.ScratchFile('WORKSPACE')
+ # All test targets are called <something>.bat, for the benefit of Windows.
+ # This makes test execution faster on Windows for the following reason:
+ #
+ # When building a sh_test rule, the main output's name is the same as the
+ # rule. On Unixes, this output is a symlink to the main script (the first
+ # entry in `srcs`), on Windows it's a copy of the file. In fact the main
+ # "script" does not have to be a script, it may be any executable file.
+ #
+ # On Unixes anything with the +x permission can be executed; the file's
+ # shebang line specifies the interpreter. On Windows, there's no such
+ # mechanism; Bazel runs the main script (which is typically a ".sh" file)
+ # through Bash. However, if the main file is a native executable, it's
+ # faster to run it directly than through Bash (plus it removes the need for
+ # Bash).
+ #
+ # Therefore on Windows, if the main script is a native executable (such as a
+ # ".bat" file) and has the same extension as the main file, Bazel (in case
+ # of sh_test) makes a copy of the file and runs it directly, rather than
+ # through Bash.
self.ScratchFile('foo/BUILD', [
'sh_test(',
' name = "passing_test.bat",',
@@ -59,7 +78,11 @@
'sh_test(',
' name = "unexported_test.bat",',
' srcs = ["unexported.bat"],',
- ' shard_count = 2,',
+ ')',
+ 'sh_test(',
+ ' name = "testargs_test.bat",',
+ ' srcs = ["testargs.bat"],',
+ ' args = ["foo", "a b", "", "bar"],',
')',
])
self.ScratchFile('foo/passing.bat', ['@exit /B 0'], executable=True)
@@ -92,6 +115,21 @@
'@echo BAD=%TEST_UNDECLARED_OUTPUTS_MANIFEST%',
],
executable=True)
+ self.ScratchFile(
+ 'foo/testargs.bat',
+ [
+ '@echo arg=(%~nx0)', # basename of $0
+ '@echo arg=(%1)',
+ '@echo arg=(%2)',
+ '@echo arg=(%3)',
+ '@echo arg=(%4)',
+ '@echo arg=(%5)',
+ '@echo arg=(%6)',
+ '@echo arg=(%7)',
+ '@echo arg=(%8)',
+ '@echo arg=(%9)',
+ ],
+ executable=True)
def _AssertPassingTest(self, flag):
exit_code, _, stderr = self.RunBazel([
@@ -229,6 +267,30 @@
if not good or bad:
self._FailWithOutput(stderr + stdout)
+ def _AssertTestArgs(self, flag, expected):
+ exit_code, bazel_bin, stderr = self.RunBazel(['info', 'bazel-bin'])
+ self.AssertExitCode(exit_code, 0, stderr)
+ bazel_bin = bazel_bin[0]
+
+ exit_code, stdout, stderr = self.RunBazel([
+ 'test',
+ '//foo:testargs_test.bat',
+ '-t-',
+ '--test_output=all',
+ '--test_arg=baz',
+ '--test_arg="x y"',
+ '--test_arg=""',
+ '--test_arg=qux',
+ flag,
+ ])
+ self.AssertExitCode(exit_code, 0, stderr)
+
+ actual = []
+ for line in stderr + stdout:
+ if line.startswith('arg='):
+ actual.append(str(line[len('arg='):]))
+ self.assertListEqual(expected, actual)
+
def testTestExecutionWithTestSetupSh(self):
self._CreateMockWorkspace()
flag = '--nowindows_native_test_wrapper'
@@ -238,6 +300,28 @@
self._AssertRunfiles(flag)
self._AssertShardedTest(flag)
self._AssertUnexportsEnvvars(flag)
+ self._AssertTestArgs(
+ flag,
+ [
+ '(testargs_test.bat)',
+ '(foo)',
+ '(a)',
+ '(b)',
+ '(bar)',
+ # Note: debugging shows that test-setup.sh receives more-or-less
+ # good arguments (let's ignore issues #6276 and #6277 for now), but
+ # mangles the last few.
+ # I (laszlocsomor@) don't know the reason (as of 2018-10-01) but
+ # since I'm planning to phase out test-setup.sh on Windows in favor
+ # of the native test wrapper, I don't intend to debug this further.
+ # The test is here merely to guard against unwanted future change of
+ # behavior.
+ '(baz)',
+ '("\\"x)',
+ '(y\\"")',
+ '("\\\\\\")',
+ '(qux")'
+ ])
def testTestExecutionWithTestWrapperExe(self):
self._CreateMockWorkspace()
@@ -251,6 +335,26 @@
self._AssertRunfiles(flag)
self._AssertShardedTest(flag)
self._AssertUnexportsEnvvars(flag)
+ self._AssertTestArgs(
+ flag,
+ [
+ '(testargs_test.bat)',
+ '(foo)',
+ # TODO(laszlocsomor): assert that "a b" is passed as one argument,
+ # not two, after https://github.com/bazelbuild/bazel/issues/6277
+ # is fixed.
+ '(a)',
+ '(b)',
+ # TODO(laszlocsomor): assert that the empty string argument is
+ # passed, after https://github.com/bazelbuild/bazel/issues/6276
+ # is fixed.
+ '(bar)',
+ '(baz)',
+ '("x y")',
+ '("")',
+ '(qux)',
+ '()'
+ ])
if __name__ == '__main__':
diff --git a/tools/test/windows/test_wrapper.cc b/tools/test/windows/test_wrapper.cc
index ce8b2bb..accdf13 100644
--- a/tools/test/windows/test_wrapper.cc
+++ b/tools/test/windows/test_wrapper.cc
@@ -28,6 +28,7 @@
#include <functional>
#include <memory>
#include <string>
+#include <vector>
#include "src/main/cpp/util/file_platform.h"
#include "src/main/cpp/util/path_platform.h"
@@ -83,6 +84,14 @@
printf("ERROR(" __FILE__ ":%d) %s\n", line, msg);
}
+void LogError(const int line, const wchar_t* msg) {
+#define _WSTR_HELPER_1(x) L##x
+#define _WSTR_HELPER_2(x) _WSTR_HELPER_1(x)
+ wprintf(L"ERROR(" _WSTR_HELPER_2(__FILE__) L":%d) %s\n", line, msg);
+#undef _WSTR_HELPER_2
+#undef _WSTR_HELPER_1
+}
+
void LogErrorWithValue(const int line, const char* msg, DWORD error_code) {
printf("ERROR(" __FILE__ ":%d) error code: %d (0x%08x): %s\n", line,
error_code, error_code, msg);
@@ -491,14 +500,84 @@
return result->Set(test_path);
}
-bool StartSubprocess(const Path& path, HANDLE* process) {
+bool AddCommandLineArg(const wchar_t* arg, const size_t arg_size,
+ const bool first, wchar_t* cmdline,
+ const size_t cmdline_limit, size_t* inout_cmdline_len) {
+ if (arg_size == 0) {
+ const size_t len = (first ? 0 : 1) + 2;
+ if (*inout_cmdline_len + len >= cmdline_limit) {
+ LogError(__LINE__,
+ (std::wstring(L"Failed to add command line argument \"") + arg +
+ L"\"; command would be too long")
+ .c_str());
+ return false;
+ }
+
+ size_t offset = *inout_cmdline_len;
+ if (!first) {
+ cmdline[offset] = L' ';
+ offset += 1;
+ }
+ cmdline[offset] = L'"';
+ cmdline[offset + 1] = L'"';
+ *inout_cmdline_len += len;
+ return true;
+ } else {
+ const size_t len = (first ? 0 : 1) + arg_size;
+ if (*inout_cmdline_len + len >= cmdline_limit) {
+ LogError(__LINE__,
+ (std::wstring(L"Failed to add command line argument \"") + arg +
+ L"\"; command would be too long")
+ .c_str());
+ return false;
+ }
+
+ size_t offset = *inout_cmdline_len;
+ if (!first) {
+ cmdline[offset] = L' ';
+ offset += 1;
+ }
+ wcsncpy(cmdline + offset, arg, arg_size);
+ offset += arg_size;
+ *inout_cmdline_len += len;
+ return true;
+ }
+}
+
+bool CreateCommandLine(const Path& path,
+ const std::vector<const wchar_t*>& args,
+ std::unique_ptr<WCHAR[]>* result) {
// kMaxCmdline value: see lpCommandLine parameter of CreateProcessW.
- static constexpr size_t kMaxCmdline = 32768;
+ static constexpr size_t kMaxCmdline = 32767;
- std::unique_ptr<WCHAR[]> cmdline(new WCHAR[kMaxCmdline]);
- size_t len = path.Get().size();
- wcsncpy(cmdline.get(), path.Get().c_str(), len + 1);
+ // Add an extra character for the final null-terminator.
+ result->reset(new WCHAR[kMaxCmdline + 1]);
+ size_t total_len = 0;
+ if (!AddCommandLineArg(path.Get().c_str(), path.Get().size(), true,
+ result->get(), kMaxCmdline, &total_len)) {
+ return false;
+ }
+
+ for (const auto arg : args) {
+ if (!AddCommandLineArg(arg, wcslen(arg), false, result->get(), kMaxCmdline,
+ &total_len)) {
+ return false;
+ }
+ }
+ // Add final null-terminator. There's surely enough room for it:
+ // AddCommandLineArg kept validating that we stay under the limit of
+ // kMaxCmdline, and the buffer is one WCHAR larger than that.
+ result->get()[total_len] = 0;
+ return true;
+}
+
+bool StartSubprocess(const Path& path, const std::vector<const wchar_t*>& args,
+ HANDLE* process) {
+ std::unique_ptr<WCHAR[]> cmdline;
+ if (!CreateCommandLine(path, args, &cmdline)) {
+ return false;
+ }
PROCESS_INFORMATION processInfo;
STARTUPINFOW startupInfo = {0};
@@ -540,7 +619,8 @@
}
bool ParseArgs(int argc, wchar_t** argv, Path* out_argv0,
- std::wstring* out_test_path_arg, bool* out_suppress_output) {
+ std::wstring* out_test_path_arg, bool* out_suppress_output,
+ std::vector<const wchar_t*>* out_args) {
if (!out_argv0->Set(argv[0])) {
return false;
}
@@ -561,12 +641,18 @@
}
*out_test_path_arg = argv[0];
+ out_args->clear();
+ out_args->reserve(argc - 1);
+ for (int i = 1; i < argc; i++) {
+ out_args->push_back(argv[i]);
+ }
return true;
}
-int RunSubprocess(const Path& test_path) {
+int RunSubprocess(const Path& test_path,
+ const std::vector<const wchar_t*>& args) {
HANDLE process;
- if (!StartSubprocess(test_path, &process)) {
+ if (!StartSubprocess(test_path, args, &process)) {
return 1;
}
Defer close_process([process]() { CloseHandle(process); });
@@ -608,7 +694,8 @@
bool suppress_output = false;
Path test_path, exec_root, srcdir, tmpdir, xml_output;
UndeclaredOutputs undecl;
- if (!ParseArgs(argc, argv, &argv0, &test_path_arg, &suppress_output) ||
+ std::vector<const wchar_t*> args;
+ if (!ParseArgs(argc, argv, &argv0, &test_path_arg, &suppress_output, &args) ||
!PrintTestLogStartMarker(suppress_output) ||
!FindTestBinary(argv0, test_path_arg, &test_path) ||
!GetCwd(&exec_root) || !ExportUserName() ||
@@ -621,5 +708,5 @@
return 1;
}
- return RunSubprocess(test_path);
+ return RunSubprocess(test_path, args);
}
