Skip to content

Commit ab630fd

Browse files
committed
test: assert the host's quote character, not a fixed one
The Windows runner failed `LocalIncludeDirsWithSpacesAreShellQuoted` for a difference that is correct: shell_quote_arg emits single quotes for POSIX sh and double for cmd.exe, and the assertion hardcoded `'`. Both the unit test and e2e 179 now take the quote character from the platform. e2e 179 also drops its `unix-shell` requirement. Paths with spaces are the Windows problem — `C:\Program Files`, an account name with a space — so skipping the test there left the platform it exists for uncovered.
1 parent 222cfe4 commit ab630fd

2 files changed

Lines changed: 12 additions & 4 deletions

File tree

tests/e2e/179_spaced_paths.sh

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
#!/usr/bin/env bash
2-
# requires: unix-shell
32
# 179_spaced_paths.sh — a project whose paths contain spaces still builds
43
#
54
# #331: two independent places assumed no path ever contains a space. On
@@ -79,7 +78,11 @@ grep -q "vendor" "$ninja_file" || {
7978
# the shell quoting wraps the whole token (so what ninja hands to sh stays one
8079
# word). The old code had only the first, which is exactly why the path
8180
# survived ninja and then split in the shell.
82-
grep -qE "'-I[^']*vendor\\\$ inc" "$ninja_file" || {
81+
# The quote character is the host shell's — POSIX sh single, cmd.exe double —
82+
# so accept either rather than pinning whichever platform this happens to run
83+
# on. (An earlier version hardcoded `'` and failed on Windows for a difference
84+
# that was correct.)
85+
grep -qE "['\"]-I[^'\"]*vendor\\\$ inc" "$ninja_file" || {
8386
echo "FAIL: include dir not shell-quoted with its prefix:"
8487
grep -oE "[^ ]*vendor[^ ]*( inc[^ ]*)?" "$ninja_file" | head -3
8588
exit 1; }

tests/unit/test_ninja_backend.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,8 +266,13 @@ TEST(NinjaBackend, LocalIncludeDirsWithSpacesAreShellQuoted) {
266266
// only the first, which is why the path survived ninja and then split in
267267
// the shell. The prefix must be INSIDE the quotes — quoting the path
268268
// alone would leave `-I` as its own word and reintroduce the split.
269-
EXPECT_NE(line.find("'-I/opt/my$ dep/include'"), std::string::npos) << line;
270-
EXPECT_NE(line.find("'-idirafter/opt/my$ dep/after'"), std::string::npos) << line;
269+
//
270+
// The quote character is the host shell's, not a fixed one: POSIX sh
271+
// wants single quotes, cmd.exe double. Hardcoding `'` passed on Linux
272+
// and failed on the Windows runner for a difference that is correct.
273+
const std::string q = mcpp::platform::is_windows ? "\"" : "'";
274+
EXPECT_NE(line.find(q + "-I/opt/my$ dep/include" + q), std::string::npos) << line;
275+
EXPECT_NE(line.find(q + "-idirafter/opt/my$ dep/after" + q), std::string::npos) << line;
271276
}
272277

273278
// #261: on Windows $local_includes is copied into a RESPONSE FILE, which the

0 commit comments

Comments
 (0)