-
Notifications
You must be signed in to change notification settings - Fork 1.7k
wslc: remove redundant short-alias E2E tests for --workdir #40192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feature/wsl-for-apps
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -101,6 +101,11 @@ static wsl::windows::common::RunningWSLCContainer CreateInternal(Session& sessio | |||||
| containerLauncher.SetUser(std::move(user)); | ||||||
| } | ||||||
|
|
||||||
| if (!options.WorkingDirectory.empty()) | ||||||
| { | ||||||
| containerLauncher.SetWorkingDirectory(std::string(options.WorkingDirectory)); | ||||||
|
||||||
| containerLauncher.SetWorkingDirectory(std::string(options.WorkingDirectory)); | |
| containerLauncher.SetWorkingDirectory(options.WorkingDirectory); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -188,6 +188,96 @@ class WSLCCLIExecutionUnitTests | |||||||||||||||||||||||||||||||||||
| VERIFY_ARE_EQUAL(std::string("/app"), options.WorkingDirectory); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // 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"); | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| ContainerRunCommand command{L""}; | ||||||||||||||||||||||||||||||||||||
| CLIExecutionContext context; | ||||||||||||||||||||||||||||||||||||
| command.ParseArguments(invocation, context.Args); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| VERIFY_THROWS_SPECIFIC( | ||||||||||||||||||||||||||||||||||||
| command.ValidateArguments(context.Args), wsl::windows::wslc::ArgumentException, [](const auto&) { return true; }); | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
| 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
AI
Apr 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
AI
Apr 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
AI
Apr 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
AI
Apr 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| 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
AI
Apr 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
AI
Apr 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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::WorkDirforcontainer run) plus additional tests/help expectations. Please update the PR title/description to match the scope (adding--workdir/-wsupport), or split the behavior change into its own PR to keep the stated intent accurate.