wslc: remove redundant short-alias E2E tests for --workdir by ptrivedi · Pull Request #40192 · microsoft/WSL

ptrivedi · 2026-04-15T18:12:48Z

Addresses review feedback from @AmelBawa-msft on #40190: removes redundant short-alias E2E tests since coverage is already provided by help message tests and CLI parse tests.

Extends the --workdir flag (already supported by container exec) to the container create and run commands, passing the working directory through to the container launcher. Adds CLI parse tests, unit tests, and E2E tests for both commands. Co-Authored-By: Claude Sonnet

Short-alias (-w) coverage is already provided by the help message test method and CLI parse tests. Remove the duplicate E2E tests for container create and run per reviewer feedback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR adds --workdir/-w support for wslc container run and wslc container create, and updates tests/help output accordingly.

Changes:

Add WorkDir argument wiring for container run and container create.
Apply the working directory to the container launcher (SetWorkingDirectory).
Add/adjust E2E tests, CLI execution unit tests, command-line test cases, and help-text expectations for --workdir/-w.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp	Adds E2E coverage for `container run --workdir` and updates expected help output.
test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp	Adds E2E coverage for `container create --workdir` and updates expected help output.
test/windows/wslc/WSLCCLIExecutionUnitTests.cpp	Adds unit tests to validate parsing/validation of `--workdir/-w` for run/create.
test/windows/wslc/CommandLineTestCases.h	Extends CLI parsing test matrix for `--workdir/-w` and invalid cases.
src/windows/wslc/services/ContainerService.cpp	Sets working directory on container launcher from options.
src/windows/wslc/commands/ContainerRunCommand.cpp	Registers `WorkDir` argument for `run`.
src/windows/wslc/commands/ContainerCreateCommand.cpp	Registers `WorkDir` argument for `create`.

Copilot · 2026-04-15T18:19:29Z

        Argument::Create(ArgType::User),
        Argument::Create(ArgType::Volume, false, NO_LIMIT),
        // Argument::Create(ArgType::Virtual),
+        Argument::Create(ArgType::WorkDir),


The PR title/description indicates this change is about removing redundant short-alias E2E tests, but this diff introduces new product behavior (wiring up ArgType::WorkDir for container run) plus additional tests/help expectations. Please update the PR title/description to match the scope (adding --workdir/-w support), or split the behavior change into its own PR to keep the stated intent accurate.

Copilot · 2026-04-15T18:19:30Z

+    // Test: Full parse of 'run --workdir "" image cmd' rejects empty working directory
+    TEST_METHOD(RunCommand_ParseWorkDirEmptyValue_ThrowsArgumentException)
+    {
+        auto invocation = CreateInvocationFromCommandLine(L"wslc --workdir \"\" ubuntu sh");


These tests are named/commented as parsing run ... and create ..., but the command lines passed to CreateInvocationFromCommandLine don’t include the run/create subcommand. This can cause the tests to exercise a different parsing path (or pass/fail for the wrong reason). Update the test command lines to include the explicit subcommand, e.g. wslc run --workdir ... and wslc create --workdir ... (and the -w variants) so the tests accurately cover the intended commands.

Copilot · 2026-04-15T18:19:30Z

+    // Test: Full parse of 'run --workdir /path image cmd' sets WorkingDirectory
+    TEST_METHOD(RunCommand_ParseWorkDirLongOption_SetsWorkingDirectory)
+    {
+        auto invocation = CreateInvocationFromCommandLine(L"wslc --workdir /tmp/mydir ubuntu sh");


These tests are named/commented as parsing run ... and create ..., but the command lines passed to CreateInvocationFromCommandLine don’t include the run/create subcommand. This can cause the tests to exercise a different parsing path (or pass/fail for the wrong reason). Update the test command lines to include the explicit subcommand, e.g. wslc run --workdir ... and wslc create --workdir ... (and the -w variants) so the tests accurately cover the intended commands.

Copilot · 2026-04-15T18:19:30Z

+    // Test: Full parse of 'run -w /path image cmd' (short alias) sets WorkingDirectory
+    TEST_METHOD(RunCommand_ParseWorkDirShortOption_SetsWorkingDirectory)
+    {
+        auto invocation = CreateInvocationFromCommandLine(L"wslc -w /app ubuntu sh");


These tests are named/commented as parsing run ... and create ..., but the command lines passed to CreateInvocationFromCommandLine don’t include the run/create subcommand. This can cause the tests to exercise a different parsing path (or pass/fail for the wrong reason). Update the test command lines to include the explicit subcommand, e.g. wslc run --workdir ... and wslc create --workdir ... (and the -w variants) so the tests accurately cover the intended commands.

Copilot · 2026-04-15T18:19:30Z

+    // Test: Full parse of 'create --workdir "" image cmd' rejects empty working directory
+    TEST_METHOD(CreateCommand_ParseWorkDirEmptyValue_ThrowsArgumentException)
+    {
+        auto invocation = CreateInvocationFromCommandLine(L"wslc --workdir \"\" ubuntu sh");


These tests are named/commented as parsing run ... and create ..., but the command lines passed to CreateInvocationFromCommandLine don’t include the run/create subcommand. This can cause the tests to exercise a different parsing path (or pass/fail for the wrong reason). Update the test command lines to include the explicit subcommand, e.g. wslc run --workdir ... and wslc create --workdir ... (and the -w variants) so the tests accurately cover the intended commands.

Copilot · 2026-04-15T18:19:30Z

+    // Test: Full parse of 'create --workdir /path image cmd' sets WorkingDirectory
+    TEST_METHOD(CreateCommand_ParseWorkDirLongOption_SetsWorkingDirectory)
+    {
+        auto invocation = CreateInvocationFromCommandLine(L"wslc --workdir /tmp/mydir ubuntu sh");


These tests are named/commented as parsing run ... and create ..., but the command lines passed to CreateInvocationFromCommandLine don’t include the run/create subcommand. This can cause the tests to exercise a different parsing path (or pass/fail for the wrong reason). Update the test command lines to include the explicit subcommand, e.g. wslc run --workdir ... and wslc create --workdir ... (and the -w variants) so the tests accurately cover the intended commands.

Copilot · 2026-04-15T18:19:31Z

+    // Test: Full parse of 'create -w /path image cmd' (short alias) sets WorkingDirectory
+    TEST_METHOD(CreateCommand_ParseWorkDirShortOption_SetsWorkingDirectory)
+    {
+        auto invocation = CreateInvocationFromCommandLine(L"wslc -w /app ubuntu sh");


These tests are named/commented as parsing run ... and create ..., but the command lines passed to CreateInvocationFromCommandLine don’t include the run/create subcommand. This can cause the tests to exercise a different parsing path (or pass/fail for the wrong reason). Update the test command lines to include the explicit subcommand, e.g. wslc run --workdir ... and wslc create --workdir ... (and the -w variants) so the tests accurately cover the intended commands.

Copilot · 2026-04-15T18:19:31Z

+        command.ParseArguments(invocation, context.Args);
+
+        VERIFY_THROWS_SPECIFIC(
+            command.ValidateArguments(context.Args), wsl::windows::wslc::ArgumentException, [](const auto&) { return true; });


The exception predicate always returns true, so these tests will pass for any ArgumentException (including ones thrown for unrelated validation failures). Make the predicate validate something specific to the --workdir empty-value case (e.g., the parameter name/flag in the message or an error code field, if available) so the test can’t succeed for the wrong reason.

Suggested change

command.ValidateArguments(context.Args), wsl::windows::wslc::ArgumentException, [](const auto&) { return true; });

command.ValidateArguments(context.Args),

wsl::windows::wslc::ArgumentException,

[](const auto& ex)

{

const std::string message = ex.what();

const bool mentionsWorkDir =

message.find("--workdir") != std::string::npos ||

message.find("workdir") != std::string::npos ||

message.find("-w") != std::string::npos;

const bool mentionsEmptyOrInvalidValue =

message.find("empty") != std::string::npos ||

message.find("invalid") != std::string::npos ||

message.find("value") != std::string::npos;

return mentionsWorkDir && mentionsEmptyOrInvalidValue;

});

Copilot · 2026-04-15T18:19:31Z

+        command.ParseArguments(invocation, context.Args);
+
+        VERIFY_THROWS_SPECIFIC(
+            command.ValidateArguments(context.Args), wsl::windows::wslc::ArgumentException, [](const auto&) { return true; });


The exception predicate always returns true, so these tests will pass for any ArgumentException (including ones thrown for unrelated validation failures). Make the predicate validate something specific to the --workdir empty-value case (e.g., the parameter name/flag in the message or an error code field, if available) so the test can’t succeed for the wrong reason.

Suggested change

command.ValidateArguments(context.Args), wsl::windows::wslc::ArgumentException, [](const auto&) { return true; });

command.ValidateArguments(context.Args),

wsl::windows::wslc::ArgumentException,

[](const auto& ex)

{

const std::string message = ex.what();

return message.find("workdir") != std::string::npos;

});

Copilot · 2026-04-15T18:19:32Z


+    if (!options.WorkingDirectory.empty())
+    {
+        containerLauncher.SetWorkingDirectory(std::string(options.WorkingDirectory));


The std::string(options.WorkingDirectory) construction is redundant if options.WorkingDirectory is already a std::string (it forces an extra copy). Prefer passing options.WorkingDirectory directly, or std::move(options.WorkingDirectory) if ownership transfer is intended and options won’t use it afterward.

Suggested change

containerLauncher.SetWorkingDirectory(std::string(options.WorkingDirectory));

containerLauncher.SetWorkingDirectory(options.WorkingDirectory);

Pooja Trivedi and others added 2 commits April 15, 2026 11:07

ptrivedi requested a review from a team as a code owner April 15, 2026 18:12

Copilot AI review requested due to automatic review settings April 15, 2026 18:12

Copilot AI reviewed Apr 15, 2026

View reviewed changes

Copilot started reviewing on behalf of ptrivedi April 15, 2026 19:37 View session

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wslc: remove redundant short-alias E2E tests for --workdir#40192

wslc: remove redundant short-alias E2E tests for --workdir#40192
ptrivedi wants to merge 2 commits intofeature/wsl-for-appsfrom
user/ptrivedi/workdir-remove-redundant-e2e

ptrivedi commented Apr 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 15, 2026

Uh oh!

Copilot AI Apr 15, 2026

Uh oh!

Copilot AI Apr 15, 2026

Uh oh!

Copilot AI Apr 15, 2026

Uh oh!

Copilot AI Apr 15, 2026

Uh oh!

Copilot AI Apr 15, 2026

Uh oh!

Copilot AI Apr 15, 2026

Uh oh!

Copilot AI Apr 15, 2026

Uh oh!

Copilot AI Apr 15, 2026

Uh oh!

Copilot AI Apr 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-            command.ValidateArguments(context.Args), wsl::windows::wslc::ArgumentException, [](const auto&) { return true; });
+            command.ValidateArguments(context.Args),
+            wsl::windows::wslc::ArgumentException,
+            [](const auto& ex)
+            {
+                const std::string message = ex.what();
+                const bool mentionsWorkDir =
+                    message.find("--workdir") != std::string::npos ||
+                    message.find("workdir") != std::string::npos ||
+                    message.find("-w") != std::string::npos;
+                const bool mentionsEmptyOrInvalidValue =
+                    message.find("empty") != std::string::npos ||
+                    message.find("invalid") != std::string::npos ||
+                    message.find("value") != std::string::npos;
+                return mentionsWorkDir && mentionsEmptyOrInvalidValue;
+            });

	containerLauncher.SetWorkingDirectory(std::string(options.WorkingDirectory));
	containerLauncher.SetWorkingDirectory(options.WorkingDirectory);

Conversation

ptrivedi commented Apr 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants