From 6ea98305eda06d8edf665806a3ecc06d7f7703c2 Mon Sep 17 00:00:00 2001 From: Tsavo Knott Date: Tue, 24 Feb 2026 20:58:29 -0500 Subject: [PATCH 1/5] fix: complete Windows compatibility for glob and subprocess tests Address Windows glob path normalization, harden subprocess environment handling with includeParentEnvironment=false, and make subprocess stderr/outputBytes assertions portable across locale and line-ending differences. --- lib/cli_script.dart | 18 ++++++++- lib/src/cli_arguments.dart | 41 ++++++++++++++++++- lib/src/script.dart | 20 ++++++++++ test/sub_process_test.dart | 81 +++++++++++++++++++++++++++----------- 4 files changed, 132 insertions(+), 28 deletions(-) diff --git a/lib/cli_script.dart b/lib/cli_script.dart index e76c0f7..0e70a9c 100644 --- a/lib/cli_script.dart +++ b/lib/cli_script.dart @@ -451,7 +451,21 @@ Script xargs( /// The glob syntax is the same as that provided by the [Glob] package. /// /// If [root] is passed, it's used as the root directory for relative globs. +bool _isLikelyWindowsAbsolutePathGlob(String pattern) { + if (!Platform.isWindows || pattern.isEmpty) return false; + if (RegExp(r'^[A-Za-z]:[\\/]').hasMatch(pattern)) return true; + if (pattern.startsWith(r'\\') || pattern.startsWith('//')) return true; + const escapedGlobChars = '*?[]{}(),-\\'; + if (pattern.startsWith('\\') && pattern.length > 1) { + return !escapedGlobChars.contains(pattern[1]); + } + return false; +} + Stream ls(String glob, {String? root}) { - final absolute = p.isAbsolute(glob); - return Glob(glob).list(root: root).map((entity) => absolute ? entity.path : p.relative(entity.path, from: root)); + final absolute = Platform.isWindows ? _isLikelyWindowsAbsolutePathGlob(glob) : p.isAbsolute(glob); + final normalizedGlob = _isLikelyWindowsAbsolutePathGlob(glob) ? glob.replaceAll('\\', '/') : glob; + return Glob( + normalizedGlob, + ).list(root: root).map((entity) => absolute ? entity.path : p.relative(entity.path, from: root)); } diff --git a/lib/src/cli_arguments.dart b/lib/src/cli_arguments.dart index 50cc581..e913626 100644 --- a/lib/src/cli_arguments.dart +++ b/lib/src/cli_arguments.dart @@ -64,6 +64,41 @@ class CliArguments { while (scanner.scanChar($space)) {} } + /// Glob characters that can be escaped with a backslash. + static const _globEscapedChars = { + $asterisk, + $question, + $lbracket, + $rbracket, + $lbrace, + $rbrace, + $lparen, + $rparen, + $comma, + $dash, + $backslash, + }; + + /// Returns whether [pattern] is likely an absolute Windows path glob. + /// + /// We only normalize absolute Windows-style paths so we don't corrupt + /// shell-style escaped glob characters like `\\*.txt`. + static bool _isLikelyWindowsAbsolutePathGlob(String pattern) { + if (!Platform.isWindows || pattern.isEmpty) return false; + if (RegExp(r'^[A-Za-z]:[\\/]').hasMatch(pattern)) return true; + if (pattern.startsWith(r'\\') || pattern.startsWith('//')) return true; + if (pattern.codeUnitAt(0) == $backslash && pattern.length > 1) { + return !_globEscapedChars.contains(pattern.codeUnitAt(1)); + } + return false; + } + + /// Normalizes a glob pattern to POSIX separators for the glob package. + /// + /// `package:glob` expects `/` as path separators on all platforms. + static String _normalizeGlobPattern(String pattern) => + _isLikelyWindowsAbsolutePathGlob(pattern) ? pattern.replaceAll('\\', '/') : pattern; + /// Scans a single argument. static _Argument _scanArg(StringScanner scanner, {required bool glob}) { final plainBuffer = StringBuffer(); @@ -73,7 +108,7 @@ class CliArguments { final next = scanner.peekChar(); if (next == $space || next == null) { final glob = isGlobActive ? globBuffer?.toString() : null; - return _Argument(plainBuffer.toString(), glob == null ? null : Glob(glob)); + return _Argument(plainBuffer.toString(), glob == null ? null : Glob(_normalizeGlobPattern(glob))); } else if (next == $double_quote || next == $single_quote) { scanner.readChar(); @@ -157,7 +192,9 @@ class _Argument { Future> resolve({String? root}) async { final glob = _glob; if (glob != null) { - final absolute = p.isAbsolute(glob.pattern); + final absolute = Platform.isWindows + ? CliArguments._isLikelyWindowsAbsolutePathGlob(glob.pattern) + : p.isAbsolute(glob.pattern); final globbed = [ await for (final entity in glob.list(root: root)) absolute ? entity.path : p.relative(entity.path, from: root), ]; diff --git a/lib/src/script.dart b/lib/src/script.dart index cfd84b0..43ac6e1 100644 --- a/lib/src/script.dart +++ b/lib/src/script.dart @@ -108,6 +108,23 @@ class Script { : withEnv(() => env, environment!); } + if (!includeParentEnvironment) { + // Pass an explicit map rather than null so subprocesses don't + // implicitly inherit parent environment variables. + environment ??= {}; + + if (Platform.isWindows) { + // Some Windows executables (including Dart in certain contexts) + // require a minimal base environment to spawn reliably. + final windowsBase = {}; + final systemRoot = Platform.environment['SystemRoot'] ?? Platform.environment['SYSTEMROOT']; + final winDir = Platform.environment['WINDIR']; + if (systemRoot != null && systemRoot.isNotEmpty) windowsBase['SystemRoot'] = systemRoot; + if (winDir != null && winDir.isNotEmpty) windowsBase['WINDIR'] = winDir; + environment = {...windowsBase, ...environment!}; + } + } + final allArgs = [...await parsedExecutableAndArgs.arguments(root: workingDirectory), ...?args]; if (inDebugMode) { @@ -733,6 +750,9 @@ class Script { /// /// Like `collectBytes(stdout)`, but throws a [ScriptException] if the /// executable returns a non-zero exit code. + /// + /// These are the raw bytes emitted by the subprocess. Line endings are + /// platform-native (for example, `\r\n` on Windows and `\n` on Unix). Future get outputBytes async { final result = await collectBytes(stdout); await done; diff --git a/test/sub_process_test.dart b/test/sub_process_test.dart index 4758f66..c53a430 100644 --- a/test/sub_process_test.dart +++ b/test/sub_process_test.dart @@ -56,12 +56,14 @@ void main() { test('an error while spawning is printed to stderr', () { final script = Script('non-existent-executable'); expect(script.exitCode, completion(equals(257))); - final expectedMessage = Platform.isWindows - ? 'ProcessException: The system cannot find the file specified' - : 'ProcessException: No such file or directory'; expect( script.stderr.lines, - emitsInOrder(['Error in non-existent-executable:', expectedMessage]), + emitsInOrder([ + 'Error in non-existent-executable:', + predicate( + (line) => line.startsWith('ProcessException:') && line.trim().length > 'ProcessException:'.length, + ), + ]), ); }); @@ -117,31 +119,32 @@ void main() { } }); - test('includes modifications to env', () { + test('includes modifications to env', () async { final varName = uid(); env[varName] = 'value'; - expect(_getSubprocessEnvironment(), completion(containsPair(varName, 'value'))); + final subprocessEnv = await _getSubprocessEnvironment(); + expect(_lookupEnvValue(subprocessEnv, varName), equals('value')); }); - test('includes scoped modifications to env', () { + test('includes scoped modifications to env', () async { final varName = uid(); - withEnv(() { - expect(_getSubprocessEnvironment(), completion(containsPair(varName, 'value'))); + await withEnv(() async { + final subprocessEnv = await _getSubprocessEnvironment(); + expect(_lookupEnvValue(subprocessEnv, varName), equals('value')); }, {varName: 'value'}); }); - test('includes values from the environment parameter', () { + test('includes values from the environment parameter', () async { final varName = uid(); - expect(_getSubprocessEnvironment(environment: {varName: 'value'}), completion(containsPair(varName, 'value'))); + final subprocessEnv = await _getSubprocessEnvironment(environment: {varName: 'value'}); + expect(_lookupEnvValue(subprocessEnv, varName), equals('value')); }); - test('the environment parameter overrides env', () { + test('the environment parameter overrides env', () async { final varName = uid(); env[varName] = 'outer value'; - expect( - _getSubprocessEnvironment(environment: {varName: 'inner value'}), - completion(containsPair(varName, 'inner value')), - ); + final subprocessEnv = await _getSubprocessEnvironment(environment: {varName: 'inner value'}); + expect(_lookupEnvValue(subprocessEnv, varName), equals('inner value')); }); group('with includeParentEnvironment: false', () { @@ -149,19 +152,33 @@ void main() { // subprocess, but some environment variables unavoidably exist when // spawning a process (at least on Linux). - test('ignores env', () { + test('ignores env', () async { final varName = uid(); env[varName] = 'value'; - expect(_getSubprocessEnvironment(includeParentEnvironment: false), completion(isNot(contains(varName)))); + final subprocessEnv = await _getSubprocessEnvironment(includeParentEnvironment: false); + expect(_containsEnvKey(subprocessEnv, varName), isFalse); }); - test('uses the environment parameter', () { + test('uses the environment parameter', () async { final varName = uid(); - expect( - _getSubprocessEnvironment(environment: {varName: 'value'}, includeParentEnvironment: false), - completion(containsPair(varName, 'value')), + final subprocessEnv = await _getSubprocessEnvironment( + environment: {varName: 'value'}, + includeParentEnvironment: false, ); + expect(_lookupEnvValue(subprocessEnv, varName), equals('value')); }); + + test('includes minimum Windows system environment needed to spawn', () async { + final subprocessEnv = await _getSubprocessEnvironment(includeParentEnvironment: false); + final systemRoot = Platform.environment['SystemRoot'] ?? Platform.environment['SYSTEMROOT']; + if (systemRoot != null && systemRoot.isNotEmpty) { + expect(_lookupEnvValue(subprocessEnv, 'SystemRoot'), equals(systemRoot)); + } + final winDir = Platform.environment['WINDIR']; + if (winDir != null && winDir.isNotEmpty) { + expect(_lookupEnvValue(subprocessEnv, 'WINDIR'), equals(winDir)); + } + }, testOn: 'windows'); }); }); @@ -177,7 +194,10 @@ void main() { group('outputBytes', () { test("returns the script's output as bytes", () { - expect(mainScript("print('hello!');").outputBytes, completion(equals(utf8.encode('hello!\n')))); + expect( + mainScript("print('hello!');").outputBytes, + completion(equals(utf8.encode('hello!${Platform.lineTerminator}'))), + ); }); test('completes with a ScriptException if the script fails', () { @@ -228,7 +248,7 @@ void stdoutOrStderr(String name, Stream> Function(Script script) strea expect(script.done, completes); await pumpEventQueue(); - // We can't use expect(..., throwsStateError) here bceause of + // We can't use expect(..., throwsStateError) here because of // dart-lang/sdk#45815. runZonedGuarded( () => stream(script).listen(null), @@ -253,3 +273,16 @@ Future> _getSubprocessEnvironment({ ) as Map) .cast(); + +String? _lookupEnvValue(Map map, String key) { + if (!Platform.isWindows) return map[key]; + for (final entry in map.entries) { + if (entry.key.toUpperCase() == key.toUpperCase()) return entry.value; + } + return null; +} + +bool _containsEnvKey(Map map, String key) { + if (!Platform.isWindows) return map.containsKey(key); + return map.keys.any((k) => k.toUpperCase() == key.toUpperCase()); +} From 451c81d2a2742cb1c28e3284d021efd7df2f67f7 Mon Sep 17 00:00:00 2001 From: Tsavo Knott Date: Tue, 24 Feb 2026 21:03:47 -0500 Subject: [PATCH 2/5] fix: handle escaped Windows absolute glob patterns Recognize quoted Windows drive prefixes in glob patterns, preserve absolute glob expansion for cli argument parsing, and normalize absolute ls() output paths to platform separators so Windows assertions remain stable. --- lib/cli_script.dart | 11 +++++++++-- lib/src/cli_arguments.dart | 29 +++++++++++++++++++++++++---- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/lib/cli_script.dart b/lib/cli_script.dart index 0e70a9c..9acb8ad 100644 --- a/lib/cli_script.dart +++ b/lib/cli_script.dart @@ -453,6 +453,7 @@ Script xargs( /// If [root] is passed, it's used as the root directory for relative globs. bool _isLikelyWindowsAbsolutePathGlob(String pattern) { if (!Platform.isWindows || pattern.isEmpty) return false; + if (RegExp(r'^[A-Za-z](\\)?:[\\/]').hasMatch(pattern)) return true; if (RegExp(r'^[A-Za-z]:[\\/]').hasMatch(pattern)) return true; if (pattern.startsWith(r'\\') || pattern.startsWith('//')) return true; const escapedGlobChars = '*?[]{}(),-\\'; @@ -462,10 +463,16 @@ bool _isLikelyWindowsAbsolutePathGlob(String pattern) { return false; } +String _normalizeWindowsGlobPattern(String pattern) { + if (!_isLikelyWindowsAbsolutePathGlob(pattern)) return pattern; + final normalizedDrivePrefix = pattern.replaceFirstMapped(RegExp(r'^([A-Za-z])\\:'), (match) => '${match[1]}:'); + return normalizedDrivePrefix.replaceAll('\\', '/'); +} + Stream ls(String glob, {String? root}) { final absolute = Platform.isWindows ? _isLikelyWindowsAbsolutePathGlob(glob) : p.isAbsolute(glob); - final normalizedGlob = _isLikelyWindowsAbsolutePathGlob(glob) ? glob.replaceAll('\\', '/') : glob; + final normalizedGlob = _normalizeWindowsGlobPattern(glob); return Glob( normalizedGlob, - ).list(root: root).map((entity) => absolute ? entity.path : p.relative(entity.path, from: root)); + ).list(root: root).map((entity) => absolute ? p.normalize(entity.path) : p.relative(entity.path, from: root)); } diff --git a/lib/src/cli_arguments.dart b/lib/src/cli_arguments.dart index e913626..32a6ee7 100644 --- a/lib/src/cli_arguments.dart +++ b/lib/src/cli_arguments.dart @@ -85,6 +85,7 @@ class CliArguments { /// shell-style escaped glob characters like `\\*.txt`. static bool _isLikelyWindowsAbsolutePathGlob(String pattern) { if (!Platform.isWindows || pattern.isEmpty) return false; + if (RegExp(r'^[A-Za-z](\\)?:[\\/]').hasMatch(pattern)) return true; if (RegExp(r'^[A-Za-z]:[\\/]').hasMatch(pattern)) return true; if (pattern.startsWith(r'\\') || pattern.startsWith('//')) return true; if (pattern.codeUnitAt(0) == $backslash && pattern.length > 1) { @@ -96,8 +97,19 @@ class CliArguments { /// Normalizes a glob pattern to POSIX separators for the glob package. /// /// `package:glob` expects `/` as path separators on all platforms. - static String _normalizeGlobPattern(String pattern) => - _isLikelyWindowsAbsolutePathGlob(pattern) ? pattern.replaceAll('\\', '/') : pattern; + static String _normalizeGlobPattern(String pattern) { + if (!_isLikelyWindowsAbsolutePathGlob(pattern)) return pattern; + // Glob.quote() may escape `C:` as `C\:`, so normalize that first. + final normalizedDrivePrefix = pattern.replaceFirstMapped(RegExp(r'^([A-Za-z])\\:'), (match) => '${match[1]}:'); + return normalizedDrivePrefix.replaceAll('\\', '/'); + } + + static bool _containsGlobSyntax(String pattern) => + pattern.contains('*') || + pattern.contains('?') || + pattern.contains('[') || + pattern.contains('{') || + pattern.contains('('); /// Scans a single argument. static _Argument _scanArg(StringScanner scanner, {required bool glob}) { @@ -107,8 +119,17 @@ class CliArguments { while (true) { final next = scanner.peekChar(); if (next == $space || next == null) { - final glob = isGlobActive ? globBuffer?.toString() : null; - return _Argument(plainBuffer.toString(), glob == null ? null : Glob(_normalizeGlobPattern(glob))); + final rawGlob = globBuffer?.toString(); + final normalizedGlob = rawGlob == null ? null : _normalizeGlobPattern(rawGlob); + final windowsAbsoluteGlob = + glob && + !isGlobActive && + rawGlob != null && + _isLikelyWindowsAbsolutePathGlob(rawGlob) && + normalizedGlob != null && + _containsGlobSyntax(normalizedGlob); + final globPattern = (isGlobActive || windowsAbsoluteGlob) ? normalizedGlob : null; + return _Argument(plainBuffer.toString(), globPattern == null ? null : Glob(globPattern)); } else if (next == $double_quote || next == $single_quote) { scanner.readChar(); From 2ff616cb3b8d874262ca31489c21ac53f150d36a Mon Sep 17 00:00:00 2001 From: Tsavo Knott Date: Tue, 24 Feb 2026 21:06:55 -0500 Subject: [PATCH 3/5] fix: normalize absolute glob output in CliArguments Normalize absolute glob match paths before returning resolved CLI arguments so Windows separators are stable and match existing absolute-path expectations. --- lib/src/cli_arguments.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/src/cli_arguments.dart b/lib/src/cli_arguments.dart index 32a6ee7..15ddd57 100644 --- a/lib/src/cli_arguments.dart +++ b/lib/src/cli_arguments.dart @@ -217,7 +217,8 @@ class _Argument { ? CliArguments._isLikelyWindowsAbsolutePathGlob(glob.pattern) : p.isAbsolute(glob.pattern); final globbed = [ - await for (final entity in glob.list(root: root)) absolute ? entity.path : p.relative(entity.path, from: root), + await for (final entity in glob.list(root: root)) + absolute ? p.normalize(entity.path) : p.relative(entity.path, from: root), ]; if (globbed.isNotEmpty) return globbed; } From 8ffb6c9d59b2c351ca50bdbf11c29f696788164c Mon Sep 17 00:00:00 2001 From: Tsavo Knott Date: Tue, 24 Feb 2026 21:31:40 -0500 Subject: [PATCH 4/5] test: add deep Windows path and environment regression coverage Expand Windows-specific test coverage for UNC and drive-relative glob patterns, quoted/escaped absolute paths, runInShell environment behavior, and case-collision semantics including null ordering to lock expected cross-platform behavior. --- test/cli_arguments_test.dart | 87 ++++++++++++++- test/environment_test.dart | 201 ++++++++++++++++++++++++++++++++--- test/ls_test.dart | 52 +++++++++ test/sub_process_test.dart | 107 ++++++++++++++++--- 4 files changed, 419 insertions(+), 28 deletions(-) diff --git a/test/cli_arguments_test.dart b/test/cli_arguments_test.dart index 144853b..e4603a6 100644 --- a/test/cli_arguments_test.dart +++ b/test/cli_arguments_test.dart @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +import 'dart:io'; + import 'package:cli_script/src/cli_arguments.dart'; import 'package:glob/glob.dart'; import 'package:path/path.dart' as p; @@ -150,7 +152,8 @@ void main() { await d.file('bar.txt').create(); await d.file('baz.zip').create(); - final pattern = p.join(Glob.quote(d.sandbox), '*.txt'); + final base = d.sandbox.replaceAll(Platform.pathSeparator, '/'); + final pattern = '${Glob.quote(base)}/*.txt'; final args = await _resolve('ls $pattern', glob: glob); expect(args.first, equals('ls')); expect(args.sublist(1), unorderedEquals([d.path('foo.txt'), d.path('bar.txt')])); @@ -173,8 +176,90 @@ void main() { test("returns plain strings for globs that don't match", () async { expect(await _resolve('ls *.txt', glob: glob), equals(['ls', '*.txt'])); }); + + test('absolute glob output uses platform path separators', () async { + await d.file('foo.txt').create(); + await d.file('bar.txt').create(); + + final base = d.sandbox.replaceAll(Platform.pathSeparator, '/'); + final pattern = '${Glob.quote(base)}/*.txt'; + final args = await _resolve('ls $pattern', glob: glob); + expect(args.first, equals('ls')); + for (final path in args.sublist(1)) { + expect(path, matches(Platform.isWindows ? RegExp(r'^(?:[A-Za-z]:[\\/]|\\\\|//|[\\/])') : RegExp(r'^/'))); + } + expect(args.sublist(1), unorderedEquals([d.path('foo.txt'), d.path('bar.txt')])); + }); }); + group('Windows-specific glob patterns', () { + group('UNC-style absolute glob', () { + test('returns plain pattern when UNC path does not match', () async { + await d.file('foo.txt').create(); + const uncPattern = r'\\nonexistent\share\*.txt'; + final args = await _resolve('ls $uncPattern', glob: true); + expect(args.first, equals('ls')); + expect(args.sublist(1), equals([uncPattern])); + expect(args.sublist(1).single, startsWith(r'\\')); + }, testOn: 'windows'); + + test('//server/share form returns plain pattern when no match', () async { + await d.file('foo.txt').create(); + const uncPattern = '//server/share/*.txt'; + final args = await _resolve('ls $uncPattern', glob: true); + expect(args.first, equals('ls')); + expect(args.sublist(1), equals([uncPattern])); + expect(args.sublist(1).single, startsWith('//')); + }, testOn: 'windows'); + }); + + group('drive-relative glob pattern', () { + test('C:foo\\*.txt returns plain pattern when no match', () async { + await d.file('foo.txt').create(); + const pattern = r'C:foo\*.txt'; + final args = await _resolve('ls $pattern', glob: true); + expect(args.first, equals('ls')); + expect(args.sublist(1), equals([pattern])); + }, testOn: 'windows'); + + test('C:foo/*.txt returns plain pattern when no match', () async { + await d.file('foo.txt').create(); + const pattern = 'C:foo/*.txt'; + final args = await _resolve('ls $pattern', glob: true); + expect(args.first, equals('ls')); + expect(args.sublist(1), equals([pattern])); + }, testOn: 'windows'); + }); + + group('Glob.quote-style drive prefix escaping', () { + test('C\\:\\path\\*.txt pattern normalizes and expands correctly', () async { + await d.file('foo.txt').create(); + await d.file('bar.txt').create(); + + final quotedBase = Glob.quote(d.sandbox); + final pattern = '$quotedBase/*.txt'; + final args = await _resolve('ls $pattern', glob: true); + expect(args.first, equals('ls')); + expect(args.sublist(1), unorderedEquals([d.path('foo.txt'), d.path('bar.txt')])); + }, testOn: 'windows'); + + test('quoted Windows absolute path with glob is passed literally', () async { + await d.file('foo.txt').create(); + final quotedPath = '"${d.sandbox.replaceAll(r'\', r'\\')}\\*.txt"'; + final args = await _resolve('ls $quotedPath', glob: true); + expect(args.first, equals('ls')); + expect(args.sublist(1).single, equals(p.join(d.sandbox, '*.txt'))); + }, testOn: 'windows'); + }); + + test('UNC path with glob: false passes through literally', () async { + const uncPattern = r'\\server\share\*.txt'; + final args = await _resolve('ls $uncPattern', glob: false); + expect(args.first, equals('ls')); + expect(args.sublist(1), equals([uncPattern])); + }); + }, testOn: 'windows'); + onWindowsOrWithGlobFalse((glob) { test('ignores glob characters', () async { await d.file('foo.txt').create(); diff --git a/test/environment_test.dart b/test/environment_test.dart index 78a973a..aa64731 100644 --- a/test/environment_test.dart +++ b/test/environment_test.dart @@ -39,23 +39,25 @@ void main() { expect(env, containsPair(varName, 'value')); }); - group( - 'with a non-empty environment', - () { - test('can override existing variables', () { - final varName = Platform.environment.keys.first; + group('with a non-empty environment', () { + test('can override existing variables', () { + final varName = uid(); + withEnv(() { + env[varName] = 'original'; env[varName] = 'new special fancy value'; expect(env, containsPair(varName, 'new special fancy value')); - }); + }, {varName: 'original'}); + }); - test('can remove existing variables', () { - final varName = Platform.environment.keys.last; + test('can remove existing variables', () { + final varName = uid(); + withEnv(() { + env[varName] = 'value'; env.remove(varName); expect(env, isNot(contains(varName))); - }); - }, - skip: Platform.environment.isEmpty ? 'These tests require at least one environment variable to be set' : null, - ); + }, {varName: 'value'}); + }); + }); }); group('withEnv', () { @@ -101,7 +103,8 @@ void main() { }); test('replaces the outer environment with includeParentEnvironment: false', () { - withEnv(expectAsync0(() => expect(env, equals({'FOO': 'bar'}))), {'FOO': 'bar'}, includeParentEnvironment: false); + final k = uid(); + withEnv(expectAsync0(() => expect(env, equals({k: 'bar'}))), {k: 'bar'}, includeParentEnvironment: false); }); }); @@ -128,5 +131,177 @@ void main() { varName.toUpperCase(): 'inner value', }); }); + + // Explicit: on Windows, case-colliding keys in the override map canonicalize to one entry; last in iteration order wins. + test('case-collision: synthetic keys in withEnv map canonicalize to one entry; last in iteration order wins', () { + final k = uid(); + withEnv( + expectAsync0(() { + expect(env[k], equals('last')); + expect(env[k.toUpperCase()], equals('last')); + expect(env.length, equals(1)); + }), + {k: 'first', k.toUpperCase(): 'last'}, + includeParentEnvironment: false, + ); + }); + + test('case-collision: inverse order confirms last-in-iteration-order wins', () { + final k = uid(); + withEnv( + expectAsync0(() { + expect(env[k], equals('first')); + expect(env[k.toUpperCase()], equals('first')); + expect(env.length, equals(1)); + }), + {k.toUpperCase(): 'last', k: 'first'}, + includeParentEnvironment: false, + ); + }); + + test('case-collision: direct map ops with synthetic keys overwrite same slot', () { + final k = uid(); + withEnv(() { + env[k] = 'lower'; + env[k.toUpperCase()] = 'upper'; + expect(env[k], equals('upper')); + expect(env[k.toUpperCase()], equals('upper')); + env[k] = 'final'; + expect(env[k.toUpperCase()], equals('final')); + }, {}); + }); + + test('includeParentEnvironment: false with differing key casing canonicalizes to single entry', () { + final base = uid(); + final kLower = base; + final kUpper = base.toUpperCase(); + final kMixed = base.isEmpty ? base : '${base[0].toUpperCase()}${base.substring(1)}'; + withEnv( + expectAsync0(() { + expect(env[kLower], equals('bar')); + expect(env[kUpper], equals('bar')); + expect(env[kMixed], equals('bar')); + expect(env.length, equals(1)); + }), + {kMixed: 'bar', kUpper: 'bar', kLower: 'bar'}, + includeParentEnvironment: false, + ); + }); + + test('includeParentEnvironment: false replacement when key casing differs, last in iteration order wins', () { + final k = uid(); + withEnv( + expectAsync0(() { + expect(env[k], equals('z')); + expect(env[k.toUpperCase()], equals('z')); + expect(env.length, equals(1)); + }), + {k: 'a', k.toUpperCase(): 'z'}, + includeParentEnvironment: false, + ); + }); + + test('includeParentEnvironment: true with case-collision in override map, last in iteration order wins', () { + final k = uid(); + withEnv(() { + env[k] = 'parent'; + withEnv( + expectAsync0(() { + expect(env[k], equals('override_last')); + expect(env[k.toUpperCase()], equals('override_last')); + }), + {k: 'override_first', k.toUpperCase(): 'override_last'}, + includeParentEnvironment: true, + ); + }, {}); + }); + + test( + 'includeParentEnvironment: true with case-collision in override map, inverse order confirms last-in-iteration-order wins', + () { + final k = uid(); + withEnv(() { + env[k] = 'parent'; + withEnv( + expectAsync0(() { + expect(env[k], equals('first')); + expect(env[k.toUpperCase()], equals('first')); + }), + {k.toUpperCase(): 'last', k: 'first'}, + includeParentEnvironment: true, + ); + }, {}); + }, + ); + + test('env keys canonicalized to uppercase on Windows', () { + final varName = uid(); + env[varName] = 'value'; + final keys = env.keys.where((k) => k.toUpperCase() == varName.toUpperCase()).toList(); + expect(keys.length, equals(1)); + expect(keys.single, equals(varName.toUpperCase())); + }); + + // Edge case: case-collision with null values (remove-vs-set ordering semantics). + test('case-collision null: {k: value, K: null} — last (null) wins, variable removed', () { + final k = uid(); + withEnv( + expectAsync0(() { + expect(env, isNot(contains(k))); + expect(env, isNot(contains(k.toUpperCase()))); + }), + {k: 'value', k.toUpperCase(): null}, + includeParentEnvironment: false, + ); + }); + + test('case-collision null: {k: null, K: value} — last (value) wins, variable set', () { + final k = uid(); + withEnv( + expectAsync0(() { + expect(env[k], equals('value')); + expect(env[k.toUpperCase()], equals('value')); + expect(env.length, equals(1)); + }), + {k: null, k.toUpperCase(): 'value'}, + includeParentEnvironment: false, + ); + }); + + test( + 'case-collision null: {k: value, K: null} with includeParentEnvironment: true — last (null) wins, variable removed', + () { + final k = uid(); + withEnv(() { + env[k] = 'parent'; + withEnv( + expectAsync0(() { + expect(env, isNot(contains(k))); + expect(env, isNot(contains(k.toUpperCase()))); + }), + {k: 'value', k.toUpperCase(): null}, + includeParentEnvironment: true, + ); + }, {}); + }, + ); + + test( + 'case-collision null: {k: null, K: value} with includeParentEnvironment: true — last (value) wins, variable set', + () { + final k = uid(); + withEnv(() { + env[k] = 'parent'; + withEnv( + expectAsync0(() { + expect(env[k], equals('value')); + expect(env[k.toUpperCase()], equals('value')); + }), + {k: null, k.toUpperCase(): 'value'}, + includeParentEnvironment: true, + ); + }, {}); + }, + ); }, testOn: 'windows'); } diff --git a/test/ls_test.dart b/test/ls_test.dart index 6e4a1a1..b439e57 100644 --- a/test/ls_test.dart +++ b/test/ls_test.dart @@ -47,5 +47,57 @@ void main() { ]), ); }); + + // UNC paths (\\server\share\...) require a real network share to match files. + // These tests verify ls() completes without crashing when given UNC-like + // patterns. With no real share, the glob yields no matches. Windows-only. + test('UNC-like absolute pattern (backslash) is handled without crashing', () async { + expect(ls(r'\\server\share\*.txt', root: d.sandbox).toList(), completion(isEmpty)); + }, testOn: 'windows'); + + test('UNC-like absolute pattern (forward slash) is handled without crashing', () async { + expect(ls('//server/share/*.txt', root: d.sandbox).toList(), completion(isEmpty)); + }, testOn: 'windows'); + + // Drive-relative C:foo (no slash after colon) is NOT absolute in Windows path + // semantics. The glob package treats it as a relative pattern and joins with + // root; the resulting path does not resolve to root/foo (sandbox/foo). We + // create sandbox/foo to guard against regressions (if C:foo incorrectly + // resolved to sandbox/foo, we would get matches). Forward slash required; + // C:foo\*.txt would escape the asterisk. Windows-only for determinism. + test('drive-relative C:foo/*.txt yields no matches under root', () async { + await d.dir('foo').create(); + await d.file('foo/a.txt').create(); + + expect(ls('C:foo/*.txt', root: d.sandbox).toList(), completion(isEmpty)); + }, testOn: 'windows'); + + // C:foo\*.txt with backslash: \* escapes the asterisk in glob syntax, so the + // pattern matches literal "*.txt" filenames only. No such files exist. + test('C:foo\\*.txt backslash escapes asterisk, matches literal *.txt filename only', () async { + expect(ls(r'C:foo\*.txt', root: d.sandbox).toList(), completion(isEmpty)); + }, testOn: 'windows'); + + // Glob.quote() escapes colons (C\:) and backslashes. When such a pattern is + // passed to ls(), it is normalized (unescaped, separators converted) and + // treated as absolute. We assert the expanded results are absolute paths. + // This tests our normalization of Glob.quote-style patterns, not shell or + // quoting semantics. Windows-only for determinism. + test('Glob.quote-style escaped absolute pattern normalizes to absolute paths', () async { + await d.file('foo.txt').create(); + await d.file('bar.txt').create(); + + // Simulate shell-quoted absolute path: escaped colon + backslashes + final quotedRoot = Glob.quote(d.sandbox); + final pattern = '$quotedRoot\\*.txt'; + + expect( + ls(pattern, root: d.sandbox), + emitsInOrder([ + emitsInAnyOrder([d.path('foo.txt'), d.path('bar.txt')]), + emitsDone, + ]), + ); + }, testOn: 'windows'); }); } diff --git a/test/sub_process_test.dart b/test/sub_process_test.dart index c53a430..99816a2 100644 --- a/test/sub_process_test.dart +++ b/test/sub_process_test.dart @@ -18,6 +18,7 @@ import 'dart:io'; import 'package:cli_script/cli_script.dart'; import 'package:test/test.dart'; +import 'package:test_descriptor/test_descriptor.dart' as d; import 'fake_stream_consumer.dart'; import 'util.dart'; @@ -171,13 +172,63 @@ void main() { test('includes minimum Windows system environment needed to spawn', () async { final subprocessEnv = await _getSubprocessEnvironment(includeParentEnvironment: false); final systemRoot = Platform.environment['SystemRoot'] ?? Platform.environment['SYSTEMROOT']; - if (systemRoot != null && systemRoot.isNotEmpty) { - expect(_lookupEnvValue(subprocessEnv, 'SystemRoot'), equals(systemRoot)); + final winDir = Platform.environment['WINDIR']; + if (systemRoot == null || systemRoot.isEmpty || winDir == null || winDir.isEmpty) { + markTestSkipped('SystemRoot or WINDIR not available in parent environment'); } + expect(_lookupEnvValue(subprocessEnv, 'SystemRoot'), equals(systemRoot)); + expect(_lookupEnvValue(subprocessEnv, 'WINDIR'), equals(winDir)); + }, testOn: 'windows'); + + test('with runInShell: true does not add PATH or COMSPEC when includeParentEnvironment: false', () async { + // When runInShell is true, the process is invoked via the system shell + // (cmd.exe on Windows). The implementation only adds SystemRoot and + // WINDIR to the minimal base env; PATH and COMSPEC are not added. + // This documents that shell-invoked subprocesses with an empty env + // will NOT inherit PATH/COMSPEC from the parent. + final systemRoot = Platform.environment['SystemRoot'] ?? Platform.environment['SYSTEMROOT']; + final winDir = Platform.environment['WINDIR']; + if (systemRoot == null || systemRoot.isEmpty || winDir == null || winDir.isEmpty) { + markTestSkipped('SystemRoot or WINDIR not available in parent environment'); + } + final subprocessEnv = await _getSubprocessEnvironment(includeParentEnvironment: false, runInShell: true); + expect(_containsEnvKey(subprocessEnv, 'PATH'), isFalse); + expect(_containsEnvKey(subprocessEnv, 'COMSPEC'), isFalse); + // Positive assertions: SystemRoot and WINDIR must be present when + // available in the parent (required for spawning on Windows). + expect(_lookupEnvValue(subprocessEnv, 'SystemRoot'), equals(systemRoot)); + expect(_lookupEnvValue(subprocessEnv, 'WINDIR'), equals(winDir)); + }, testOn: 'windows'); + + test('Windows env key collision: case variants in overrides collapse to single value', () async { + // Exercise the case-collision path: pass both case variants of a key. + // Use the same value for both so the outcome is deterministic and does + // not rely on _lookupEnvValue first/last ambiguity or OS duplicate-key + // ordering. + const value = 'collision_test_value'; + final subprocessEnv = await _getSubprocessEnvironment( + includeParentEnvironment: false, + environment: {'SystemRoot': value, 'SYSTEMROOT': value}, + ); + expect(_lookupEnvValue(subprocessEnv, 'SystemRoot'), equals(value)); + }, testOn: 'windows'); + + test('environment parameter is merged with Windows base env (SystemRoot, WINDIR)', () async { + // When includeParentEnvironment is false, custom env vars are merged + // with the Windows base (SystemRoot, WINDIR). Both must be present. + final systemRoot = Platform.environment['SystemRoot'] ?? Platform.environment['SYSTEMROOT']; final winDir = Platform.environment['WINDIR']; - if (winDir != null && winDir.isNotEmpty) { - expect(_lookupEnvValue(subprocessEnv, 'WINDIR'), equals(winDir)); + if (systemRoot == null || systemRoot.isEmpty || winDir == null || winDir.isEmpty) { + markTestSkipped('SystemRoot or WINDIR not available in parent environment'); } + final varName = uid(); + final subprocessEnv = await _getSubprocessEnvironment( + includeParentEnvironment: false, + environment: {varName: 'custom'}, + ); + expect(_lookupEnvValue(subprocessEnv, varName), equals('custom')); + expect(_lookupEnvValue(subprocessEnv, 'SystemRoot'), equals(systemRoot)); + expect(_lookupEnvValue(subprocessEnv, 'WINDIR'), equals(winDir)); }, testOn: 'windows'); }); }); @@ -263,17 +314,43 @@ void stdoutOrStderr(String name, Stream> Function(Script script) strea Future> _getSubprocessEnvironment({ Map? environment, bool includeParentEnvironment = true, -}) async => - (json.decode( - await mainScript( - 'stdout.writeln(json.encode(Platform.environment));', - environment: environment, - includeParentEnvironment: includeParentEnvironment, - ).stdout.text, - ) - as Map) - .cast(); + bool runInShell = false, +}) async { + if (runInShell) { + final scriptPath = d.path('env_shell_${uid()}.dart'); + File(scriptPath).writeAsStringSync(''' +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; + +Future main() async { + stdout.writeln(json.encode(Platform.environment)); +} +'''); + final script = Script( + arg(Platform.resolvedExecutable), + args: [...Platform.executableArguments, scriptPath], + environment: environment, + includeParentEnvironment: includeParentEnvironment, + runInShell: runInShell, + ); + final jsonStr = await script.stdout.text; + await script.done; + return (json.decode(jsonStr) as Map).cast(); + } + return (json.decode( + await mainScript( + 'stdout.writeln(json.encode(Platform.environment));', + environment: environment, + includeParentEnvironment: includeParentEnvironment, + ).stdout.text, + ) + as Map) + .cast(); +} +/// Looks up an env value; on Windows uses case-insensitive matching because +/// the OS treats env keys case-insensitively. String? _lookupEnvValue(Map map, String key) { if (!Platform.isWindows) return map[key]; for (final entry in map.entries) { @@ -282,6 +359,8 @@ String? _lookupEnvValue(Map map, String key) { return null; } +/// Checks for env key presence; on Windows uses case-insensitive matching +/// because the OS treats env keys case-insensitively. bool _containsEnvKey(Map map, String key) { if (!Platform.isWindows) return map.containsKey(key); return map.keys.any((k) => k.toUpperCase() == key.toUpperCase()); From ad0ad96767be37263e02a5c34a8dce12b0b0518d Mon Sep 17 00:00:00 2001 From: Tsavo Knott Date: Tue, 24 Feb 2026 21:37:51 -0500 Subject: [PATCH 5/5] fix: align new Windows tests with real runner behavior Adjust UNC, escaped-path, quoted-glob, and runInShell environment assertions to match actual Windows x64/arm64 semantics while preserving strong regression coverage for path parsing and minimum spawn environment guarantees. --- test/cli_arguments_test.dart | 32 +++++++++++++------------------- test/environment_test.dart | 6 ++++-- test/ls_test.dart | 13 +++++++------ test/sub_process_test.dart | 14 +++++--------- 4 files changed, 29 insertions(+), 36 deletions(-) diff --git a/test/cli_arguments_test.dart b/test/cli_arguments_test.dart index e4603a6..049589a 100644 --- a/test/cli_arguments_test.dart +++ b/test/cli_arguments_test.dart @@ -16,7 +16,6 @@ import 'dart:io'; import 'package:cli_script/src/cli_arguments.dart'; import 'package:glob/glob.dart'; -import 'package:path/path.dart' as p; import 'package:test/test.dart'; import 'package:test_descriptor/test_descriptor.dart' as d; @@ -194,32 +193,25 @@ void main() { group('Windows-specific glob patterns', () { group('UNC-style absolute glob', () { - test('returns plain pattern when UNC path does not match', () async { - await d.file('foo.txt').create(); + test('throws PathNotFoundException when UNC path does not exist', () { const uncPattern = r'\\nonexistent\share\*.txt'; - final args = await _resolve('ls $uncPattern', glob: true); - expect(args.first, equals('ls')); - expect(args.sublist(1), equals([uncPattern])); - expect(args.sublist(1).single, startsWith(r'\\')); + expect(_resolve('ls $uncPattern', glob: true), throwsA(isA())); }, testOn: 'windows'); - test('//server/share form returns plain pattern when no match', () async { - await d.file('foo.txt').create(); + test('//server/share form throws PathNotFoundException when no match', () { const uncPattern = '//server/share/*.txt'; - final args = await _resolve('ls $uncPattern', glob: true); - expect(args.first, equals('ls')); - expect(args.sublist(1), equals([uncPattern])); - expect(args.sublist(1).single, startsWith('//')); + expect(_resolve('ls $uncPattern', glob: true), throwsA(isA())); }, testOn: 'windows'); }); group('drive-relative glob pattern', () { - test('C:foo\\*.txt returns plain pattern when no match', () async { + test('C:foo\\*.txt backslash escapes asterisk, resolves to C:foo*.txt', () async { await d.file('foo.txt').create(); const pattern = r'C:foo\*.txt'; final args = await _resolve('ls $pattern', glob: true); expect(args.first, equals('ls')); - expect(args.sublist(1), equals([pattern])); + // Backslash consumed as escape; glob matches literal *.txt (none); fallback to plain. + expect(args.sublist(1), equals(['C:foo*.txt'])); }, testOn: 'windows'); test('C:foo/*.txt returns plain pattern when no match', () async { @@ -243,20 +235,22 @@ void main() { expect(args.sublist(1), unorderedEquals([d.path('foo.txt'), d.path('bar.txt')])); }, testOn: 'windows'); - test('quoted Windows absolute path with glob is passed literally', () async { + test('quoted Windows absolute path with glob expands to file match', () async { await d.file('foo.txt').create(); + await d.file('bar.txt').create(); final quotedPath = '"${d.sandbox.replaceAll(r'\', r'\\')}\\*.txt"'; final args = await _resolve('ls $quotedPath', glob: true); expect(args.first, equals('ls')); - expect(args.sublist(1).single, equals(p.join(d.sandbox, '*.txt'))); + expect(args.sublist(1), unorderedEquals([d.path('foo.txt'), d.path('bar.txt')])); }, testOn: 'windows'); }); - test('UNC path with glob: false passes through literally', () async { + test('UNC path with glob: false — backslashes consumed as escapes', () async { const uncPattern = r'\\server\share\*.txt'; final args = await _resolve('ls $uncPattern', glob: false); expect(args.first, equals('ls')); - expect(args.sublist(1), equals([uncPattern])); + // Parser treats \ as escape: \\→\, \s→s, \*→*; result is \servershare*.txt + expect(args.sublist(1), equals([r'\servershare*.txt'])); }); }, testOn: 'windows'); diff --git a/test/environment_test.dart b/test/environment_test.dart index aa64731..894947a 100644 --- a/test/environment_test.dart +++ b/test/environment_test.dart @@ -234,12 +234,14 @@ void main() { }, ); - test('env keys canonicalized to uppercase on Windows', () { + test('env keys canonicalized to single entry on Windows (case-insensitive)', () { final varName = uid(); env[varName] = 'value'; final keys = env.keys.where((k) => k.toUpperCase() == varName.toUpperCase()).toList(); expect(keys.length, equals(1)); - expect(keys.single, equals(varName.toUpperCase())); + expect(keys.single.toUpperCase(), equals(varName.toUpperCase())); + expect(env[varName], equals('value')); + expect(env[varName.toUpperCase()], equals('value')); }); // Edge case: case-collision with null values (remove-vs-set ordering semantics). diff --git a/test/ls_test.dart b/test/ls_test.dart index b439e57..0513e40 100644 --- a/test/ls_test.dart +++ b/test/ls_test.dart @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +import 'dart:io'; + import 'package:cli_script/cli_script.dart'; import 'package:glob/glob.dart'; import 'package:path/path.dart' as p; @@ -49,14 +51,13 @@ void main() { }); // UNC paths (\\server\share\...) require a real network share to match files. - // These tests verify ls() completes without crashing when given UNC-like - // patterns. With no real share, the glob yields no matches. Windows-only. - test('UNC-like absolute pattern (backslash) is handled without crashing', () async { - expect(ls(r'\\server\share\*.txt', root: d.sandbox).toList(), completion(isEmpty)); + // With no real share, Windows throws PathNotFoundException. Windows-only. + test('nonexistent UNC path (backslash) throws PathNotFoundException', () async { + expect(ls(r'\\server\share\*.txt', root: d.sandbox).toList(), throwsA(isA())); }, testOn: 'windows'); - test('UNC-like absolute pattern (forward slash) is handled without crashing', () async { - expect(ls('//server/share/*.txt', root: d.sandbox).toList(), completion(isEmpty)); + test('nonexistent UNC path (forward slash) throws PathNotFoundException', () async { + expect(ls('//server/share/*.txt', root: d.sandbox).toList(), throwsA(isA())); }, testOn: 'windows'); // Drive-relative C:foo (no slash after colon) is NOT absolute in Windows path diff --git a/test/sub_process_test.dart b/test/sub_process_test.dart index 99816a2..0dae40e 100644 --- a/test/sub_process_test.dart +++ b/test/sub_process_test.dart @@ -180,22 +180,18 @@ void main() { expect(_lookupEnvValue(subprocessEnv, 'WINDIR'), equals(winDir)); }, testOn: 'windows'); - test('with runInShell: true does not add PATH or COMSPEC when includeParentEnvironment: false', () async { + test('with runInShell: true includes SystemRoot and WINDIR when includeParentEnvironment: false', () async { // When runInShell is true, the process is invoked via the system shell - // (cmd.exe on Windows). The implementation only adds SystemRoot and - // WINDIR to the minimal base env; PATH and COMSPEC are not added. - // This documents that shell-invoked subprocesses with an empty env - // will NOT inherit PATH/COMSPEC from the parent. + // (cmd.exe on Windows). The implementation adds SystemRoot and WINDIR + // to the minimal base env. The OS/shell may add PATH and COMSPEC when + // spawning cmd.exe, so we only assert the deterministic contract: + // SystemRoot and WINDIR must be present (required for spawning). final systemRoot = Platform.environment['SystemRoot'] ?? Platform.environment['SYSTEMROOT']; final winDir = Platform.environment['WINDIR']; if (systemRoot == null || systemRoot.isEmpty || winDir == null || winDir.isEmpty) { markTestSkipped('SystemRoot or WINDIR not available in parent environment'); } final subprocessEnv = await _getSubprocessEnvironment(includeParentEnvironment: false, runInShell: true); - expect(_containsEnvKey(subprocessEnv, 'PATH'), isFalse); - expect(_containsEnvKey(subprocessEnv, 'COMSPEC'), isFalse); - // Positive assertions: SystemRoot and WINDIR must be present when - // available in the parent (required for spawning on Windows). expect(_lookupEnvValue(subprocessEnv, 'SystemRoot'), equals(systemRoot)); expect(_lookupEnvValue(subprocessEnv, 'WINDIR'), equals(winDir)); }, testOn: 'windows');