Skip to content

Commit

Permalink
Windows, JNI: add tests for windows_util
Browse files Browse the repository at this point in the history
Add tests for the AsExecutablePathForCreateProcess
method, since its logic is pretty complex.

Unfortunately testing it also requires complex
logic, as we need to test what exactly happens
when the input path is shorter than MAX_PATH or
when it's longer than it. To test that reliably,
we need a base path that we know will not get
shortened. Creating that base path under the temp
directory is a nightmare, we need to:
(1) retrieve the temp dir, shorten it so we know
that it won't be shortened further
(2) keep creating subdirectories that have a short
name so they also won't get shortened, but keep
the entire path below MAX_PATH while leaving
enough space for a file name in the end
(3) append a file name such that the path is just
below MAX_PATH, or is exactly that long, or is
longer than it. Because of steps (1) and (2) we
can be sure that no other component in the path
will get shortened, so we can test exactly what's
going on with the shortener logic and its error
handling. But oh boy is it complicated.

Side note, we need to use the Widechar WinAPI
functions to create/delete the directories and
files, because the POSIX API on Windows appears to
be backed by the ASCII API functions, so
attempting to `mkdir` with a path longer than
CreateDirectoryA's limit is going to fail.

But on the positive side, adding tests caught two
bugs in the method, so we have that going for us
which is nice.

See #2107
See #2181

--
PiperOrigin-RevId: 144823029
MOS_MIGRATED_REVID=144823029
  • Loading branch information
laszlocsomor authored and vladmos committed Jan 18, 2017
1 parent 63c9af4 commit 73e971c
Show file tree
Hide file tree
Showing 6 changed files with 399 additions and 6 deletions.
1 change: 1 addition & 0 deletions src/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,7 @@ filegroup(
"//src/objc_tools/plmerge:srcs",
"//src/objc_tools/xcodegen:srcs",
"//src/test/cpp:srcs",
"//src/test/native:srcs",
"//src/test/java/com/google/devtools/build/android:srcs",
"//src/test/java/com/google/devtools/build/docgen:srcs",
"//src/test/java/com/google/devtools/build/lib:srcs",
Expand Down
7 changes: 7 additions & 0 deletions src/main/native/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,13 @@ cc_binary(
],
)

cc_library(
name = "windows_jni_utils",
srcs = ["windows_util.cc"],
hdrs = ["windows_util.h"],
visibility = ["//src/test/native:__pkg__"],
)

genrule(
name = "windows_jni",
srcs = glob([
Expand Down
8 changes: 4 additions & 4 deletions src/main/native/windows_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,10 @@ string AsExecutablePathForCreateProcess(const string& path,
}

if (wshort_size >= kMaxShortPath) {
return windows_util::GetLastErrorString(
string("GetShortPathName would not shorten the path enough (path=") +
path + ")");
return string("GetShortPathName would not shorten the path enough (path=") +
path + ")";
}
GetShortPathNameW(wlong.c_str(), wshort, kMaxShortPath);

// Convert the result to UTF-8.
char mbs_short[MAX_PATH];
Expand All @@ -137,7 +137,7 @@ string AsExecutablePathForCreateProcess(const string& path,
if (mbs_size < 0 || mbs_size >= MAX_PATH) {
return string("wcstombs failed (path=") + path + ")";
}
mbs_short[mbs_size - 1] = 0;
mbs_short[mbs_size] = 0;

QuotePath(mbs_short, result);
return "";
Expand Down
2 changes: 0 additions & 2 deletions src/main/native/windows_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
#ifndef BAZEL_SRC_MAIN_NATIVE_WINDOWS_UTIL_H__
#define BAZEL_SRC_MAIN_NATIVE_WINDOWS_UTIL_H__

#include <jni.h>

#include <functional>
#include <memory>
#include <string>
Expand Down
20 changes: 20 additions & 0 deletions src/test/native/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Description:
# C++ utility tests for Bazel
package(default_visibility = ["//visibility:public"])

filegroup(
name = "srcs",
srcs = glob(["**"]),
visibility = ["//src:__pkg__"],
)

cc_test(
name = "windows_util_test",
srcs = ["windows_util_test.cc"],
deps = [
"//src/main/native:windows_jni_utils",
"//third_party:gtest",
],
)

test_suite(name = "all_tests")
Loading

0 comments on commit 73e971c

Please sign in to comment.