From fbc2aebfbc9a59035f26a1d6ba1e6afd13b9eb6b Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Wed, 18 Mar 2026 15:28:57 -0700 Subject: [PATCH 01/17] Init env --- src/windows/wslc/services/ContainerModel.h | 1 + src/windows/wslc/services/ContainerService.cpp | 2 +- src/windows/wslc/tasks/ContainerTasks.cpp | 10 ++++++++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/windows/wslc/services/ContainerModel.h b/src/windows/wslc/services/ContainerModel.h index fe1cdd463..1dcf5925a 100644 --- a/src/windows/wslc/services/ContainerModel.h +++ b/src/windows/wslc/services/ContainerModel.h @@ -30,6 +30,7 @@ enum class FormatType struct ContainerOptions { std::vector Arguments; + std::vector EnvironmentVariables; bool Detach = false; bool Interactive = false; std::string Name; diff --git a/src/windows/wslc/services/ContainerService.cpp b/src/windows/wslc/services/ContainerService.cpp index b2e30a6f2..b4f286d8b 100644 --- a/src/windows/wslc/services/ContainerService.cpp +++ b/src/windows/wslc/services/ContainerService.cpp @@ -84,7 +84,7 @@ static wsl::windows::common::RunningWSLAContainer CreateInternal( WI_SetFlagIf(containerFlags, WSLAContainerFlagsRm, options.Remove); wsl::windows::common::WSLAContainerLauncher containerLauncher( - image, options.Name, options.Arguments, {}, WSLAContainerNetworkTypeHost, processFlags); + image, options.Name, options.Arguments, options.EnvironmentVariables, WSLAContainerNetworkTypeHost, processFlags); containerLauncher.SetContainerFlags(containerFlags); auto [result, runningContainer] = containerLauncher.CreateNoThrow(*session.Get()); diff --git a/src/windows/wslc/tasks/ContainerTasks.cpp b/src/windows/wslc/tasks/ContainerTasks.cpp index d1286a11c..ceaf42410 100644 --- a/src/windows/wslc/tasks/ContainerTasks.cpp +++ b/src/windows/wslc/tasks/ContainerTasks.cpp @@ -215,6 +215,16 @@ void SetContainerOptionsFromArgs(CLIExecutionContext& context) options.Arguments.emplace_back(WideToMultiByte(context.Args.Get())); } + if (context.Args.Contains(ArgType::Env)) + { + auto const& envArgs = context.Args.GetAll(); + options.EnvironmentVariables.reserve(options.EnvironmentVariables.size() + envArgs.size()); + for (const auto& arg : envArgs) + { + options.EnvironmentVariables.emplace_back(WideToMultiByte(arg)); + } + } + if (context.Args.Contains(ArgType::ForwardArgs)) { auto const& forwardArgs = context.Args.Get(); From 596b4dd24283a782d0265c1b8f1a533443ffa0e4 Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Wed, 18 Mar 2026 17:17:55 -0700 Subject: [PATCH 02/17] Added E2E tests --- .../wslc/e2e/WSLCE2EContainerCreateTests.cpp | 122 ++++++++++++++++++ 1 file changed, 122 insertions(+) diff --git a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp index aa5405978..b42099f06 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp @@ -25,12 +25,19 @@ class WSLCE2EContainerCreateTests TEST_CLASS_SETUP(ClassSetup) { EnsureImageIsLoaded(DebianImage); + + VERIFY_IS_TRUE(::SetEnvironmentVariableW(HostEnvVariableName.c_str(), HostEnvVariableValue.c_str())); + VERIFY_IS_TRUE(::SetEnvironmentVariableW(MissingHostEnvVariableName.c_str(), nullptr)); return true; } TEST_CLASS_CLEANUP(ClassCleanup) { EnsureContainerDoesNotExist(WslcContainerName); + EnsureImageIsDeleted(DebianImage); + + VERIFY_IS_TRUE(::SetEnvironmentVariableW(HostEnvVariableName.c_str(), nullptr)); + VERIFY_IS_TRUE(::SetEnvironmentVariableW(MissingHostEnvVariableName.c_str(), nullptr)); return true; } @@ -127,8 +134,123 @@ class WSLCE2EContainerCreateTests VerifyContainerIsNotListed(WslcContainerName); } + TEST_METHOD(WSLCE2E_Container_Run_EnvOption) + { + WSL2_TEST_ONLY(); + VerifyContainerIsNotListed(WslcContainerName); + + auto result = + RunWslc(std::format(L"container run --rm --name {} -e {}=A {} env", WslcContainerName, HostEnvVariableName, DebianImage.NameAndTag())); + result.Verify({.Stderr = L"", .ExitCode = S_OK}); + + const auto outputLines = result.GetStdoutLines(); + bool foundEnv = false; + for (const auto& line : outputLines) + { + if (line == std::format(L"{}=A", HostEnvVariableName)) + { + foundEnv = true; + break; + } + } + + VERIFY_IS_TRUE(foundEnv); + } + + TEST_METHOD(WSLCE2E_Container_Run_EnvOption_MultipleValues) + { + WSL2_TEST_ONLY(); + VerifyContainerIsNotListed(WslcContainerName); + + auto result = RunWslc(std::format( + L"container run --rm --name {} -e {}=A -e {}=B {} env", + WslcContainerName, + HostEnvVariableName, + HostEnvVariableName2, + DebianImage.NameAndTag())); + result.Verify({.Stderr = L"", .ExitCode = S_OK}); + + bool foundFirstEnv = false; + bool foundSecondEnv = false; + for (const auto& line : result.GetStdoutLines()) + { + if (line == std::format(L"{}=A", HostEnvVariableName)) + { + foundFirstEnv = true; + } + else if (line == std::format(L"{}=B", HostEnvVariableName2)) + { + foundSecondEnv = true; + } + } + + VERIFY_IS_TRUE(foundFirstEnv); + VERIFY_IS_TRUE(foundSecondEnv); + } + + TEST_METHOD(WSLCE2E_Container_Run_EnvOption_KeyOnly_UsesHostValue) + { + // https://github.com/microsoft/WSL/issues/14474 + SKIP_TEST_NOT_IMPL(); + + WSL2_TEST_ONLY(); + VerifyContainerIsNotListed(WslcContainerName); + + auto result = RunWslc(std::format( + L"container run --rm --name {} -e {} {} env", + WslcContainerName, + HostEnvVariableName, + DebianImage.NameAndTag())); + result.Dump(); + result.Verify({.Stderr = L"", .ExitCode = S_OK}); + + const auto expectedLine = std::format(L"{}={}", HostEnvVariableName, HostEnvVariableValue); + bool foundEnv = false; + for (const auto& line : result.GetStdoutLines()) + { + if (line == expectedLine) + { + foundEnv = true; + break; + } + } + + VERIFY_IS_TRUE(foundEnv); + } + + TEST_METHOD(WSLCE2E_Container_Run_EnvOption_KeyOnly_HostVariableMissing) + { + WSL2_TEST_ONLY(); + VerifyContainerIsNotListed(WslcContainerName); + + auto result = RunWslc(std::format( + L"container run --rm --name {} -e {} {} env", + WslcContainerName, + MissingHostEnvVariableName, + DebianImage.NameAndTag())); + result.Verify({.Stderr = L"", .ExitCode = S_OK}); + + const auto prefix = std::format(L"{}=", MissingHostEnvVariableName); + bool foundEnv = false; + for (const auto& line : result.GetStdoutLines()) + { + if (line.rfind(prefix, 0) == 0) + { + foundEnv = true; + break; + } + } + + VERIFY_IS_FALSE(foundEnv); + } + private: const std::wstring WslcContainerName = L"wslc-test-container"; + const std::wstring HostEnvVariableName = L"WSLC_TEST_HOST_ENV"; + const std::wstring HostEnvVariableName2 = L"WSLC_TEST_HOST_ENV2"; + const std::wstring HostEnvVariableValue = L"wslc-host-env-value"; + const std::wstring HostEnvVariableValue2 = L"wslc-host-env-value2"; + const std::wstring MissingHostEnvVariableName = L"WSLC_TEST_MISSING_HOST_ENV"; const TestImage& DebianImage = DebianTestImage(); const TestImage& InvalidImage = InvalidTestImage(); From e802f4f732e70b5bf0e4595508b60d7436b17b3c Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Wed, 18 Mar 2026 17:18:26 -0700 Subject: [PATCH 03/17] Clang format --- .../wslc/e2e/WSLCE2EContainerCreateTests.cpp | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp index b42099f06..45b654592 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp @@ -139,8 +139,8 @@ class WSLCE2EContainerCreateTests WSL2_TEST_ONLY(); VerifyContainerIsNotListed(WslcContainerName); - auto result = - RunWslc(std::format(L"container run --rm --name {} -e {}=A {} env", WslcContainerName, HostEnvVariableName, DebianImage.NameAndTag())); + auto result = RunWslc(std::format( + L"container run --rm --name {} -e {}=A {} env", WslcContainerName, HostEnvVariableName, DebianImage.NameAndTag())); result.Verify({.Stderr = L"", .ExitCode = S_OK}); const auto outputLines = result.GetStdoutLines(); @@ -197,10 +197,7 @@ class WSLCE2EContainerCreateTests VerifyContainerIsNotListed(WslcContainerName); auto result = RunWslc(std::format( - L"container run --rm --name {} -e {} {} env", - WslcContainerName, - HostEnvVariableName, - DebianImage.NameAndTag())); + L"container run --rm --name {} -e {} {} env", WslcContainerName, HostEnvVariableName, DebianImage.NameAndTag())); result.Dump(); result.Verify({.Stderr = L"", .ExitCode = S_OK}); @@ -224,10 +221,7 @@ class WSLCE2EContainerCreateTests VerifyContainerIsNotListed(WslcContainerName); auto result = RunWslc(std::format( - L"container run --rm --name {} -e {} {} env", - WslcContainerName, - MissingHostEnvVariableName, - DebianImage.NameAndTag())); + L"container run --rm --name {} -e {} {} env", WslcContainerName, MissingHostEnvVariableName, DebianImage.NameAndTag())); result.Verify({.Stderr = L"", .ExitCode = S_OK}); const auto prefix = std::format(L"{}=", MissingHostEnvVariableName); From d7fd8cda8aef486d6e37f57ad5a7fb555186f1eb Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Thu, 19 Mar 2026 10:51:22 -0700 Subject: [PATCH 04/17] WIP --- src/windows/wslc/services/ContainerModel.cpp | 103 +++++++++++++++++++ src/windows/wslc/services/ContainerModel.h | 6 ++ src/windows/wslc/tasks/ContainerTasks.cpp | 10 ++ 3 files changed, 119 insertions(+) create mode 100644 src/windows/wslc/services/ContainerModel.cpp diff --git a/src/windows/wslc/services/ContainerModel.cpp b/src/windows/wslc/services/ContainerModel.cpp new file mode 100644 index 000000000..d4fb35323 --- /dev/null +++ b/src/windows/wslc/services/ContainerModel.cpp @@ -0,0 +1,103 @@ +/*++ + +Copyright (c) Microsoft. All rights reserved. + +Module Name: + + ContainerModel.cpp + +Abstract: + + This file contains the ContainerModel implementation +--*/ + +#include "precomp.h" +#include "ContainerModel.h" +#include + +namespace wsl::windows::wslc::models { + +static inline bool IsSpace(char c) +{ + return std::isspace(static_cast(c)); +}; + +std::optional EnvironmentVariable::Parse(std::string entry) +{ + if(entry.empty() || std::all_of(entry.begin(), entry.end(), IsSpace)) + { + return std::nullopt; + } + + std::string key; + std::optional value; + + auto delimiterPos = entry.find('='); + if (delimiterPos == std::string::npos) + { + key = entry; + } + else + { + key = entry.substr(0, delimiterPos); + value = entry.substr(delimiterPos + 1); + } + + if (key.empty()) + { + THROW_HR_WITH_USER_ERROR(E_INVALIDARG, "Environment variable key cannot be empty"); + } + + if (std::any_of(key.begin(), key.end(), IsSpace)) + { + THROW_HR_WITH_USER_ERROR(E_INVALIDARG, std::format(L"Environment variable key '{}' cannot contain whitespace", key)); + } + + if (!value.has_value()) + { + const auto envValue = std::getenv(key.c_str()); + if (envValue == nullptr) + { + return std::nullopt; + } + + value = std::string(envValue); + } + + return std::format("{}={}", key, value.value()); +} + +std::vector EnvironmentVariable::ParseFile(std::string filePath) +{ + if (!std::filesystem::exists(filePath)) + { + throw std::invalid_argument(std::format("Environment file '{}' does not exist", filePath)); + } + + // Read the file line by line + std::vector envVars; + std::ifstream file(filePath); + std::string line; + while (std::getline(file, line)) + { + // Remove leading whitespace + line.erase(line.begin(), std::find_if(line.begin(), line.end(), [](unsigned char ch) { + return !std::isspace(ch); + })); + + // Skip empty lines and comments + if (line.empty() || line[0] == '#') + { + continue; + } + + auto envVar = Parse(line); + if (envVar.has_value()) + { + envVars.push_back(std::move(envVar.value())); + } + } + + return envVars; +} +} // namespace wsl::windows::wslc::models diff --git a/src/windows/wslc/services/ContainerModel.h b/src/windows/wslc/services/ContainerModel.h index fe1cdd463..79d9a8c01 100644 --- a/src/windows/wslc/services/ContainerModel.h +++ b/src/windows/wslc/services/ContainerModel.h @@ -66,4 +66,10 @@ struct ContainerInformation NLOHMANN_DEFINE_TYPE_INTRUSIVE_ONLY_SERIALIZE(ContainerInformation, Id, Name, Image, State, StateChangedAt, CreatedAt); }; + +struct EnvironmentVariable +{ + static std::optional Parse(std::string entry); + static std::vector ParseFile(std::string filePath); +}; } // namespace wsl::windows::wslc::models diff --git a/src/windows/wslc/tasks/ContainerTasks.cpp b/src/windows/wslc/tasks/ContainerTasks.cpp index d1286a11c..d82c4f1c8 100644 --- a/src/windows/wslc/tasks/ContainerTasks.cpp +++ b/src/windows/wslc/tasks/ContainerTasks.cpp @@ -215,6 +215,16 @@ void SetContainerOptionsFromArgs(CLIExecutionContext& context) options.Arguments.emplace_back(WideToMultiByte(context.Args.Get())); } + if (context.Args.Contains(ArgType::EnvFile)) + { + auto const& envFiles = context.Args.GetAll(); + for (const auto& envFile : envFiles) + { + auto parsedEnvVars = EnvironmentVariable::ParseFile(WideToMultiByte(envFile)); + options.EnvironmentVariables.insert(options.EnvironmentVariables.end(), parsedEnvVars.begin(), parsedEnvVars.end()); + } + } + if (context.Args.Contains(ArgType::ForwardArgs)) { auto const& forwardArgs = context.Args.Get(); From 52e9363cdfbaa3885c261ad4bcf770c74c6c52cf Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Thu, 19 Mar 2026 11:44:24 -0700 Subject: [PATCH 05/17] Init tests --- src/windows/wslc/tasks/ContainerTasks.cpp | 6 +++++- test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp | 4 ---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/windows/wslc/tasks/ContainerTasks.cpp b/src/windows/wslc/tasks/ContainerTasks.cpp index 1122809a9..77349fc2d 100644 --- a/src/windows/wslc/tasks/ContainerTasks.cpp +++ b/src/windows/wslc/tasks/ContainerTasks.cpp @@ -230,7 +230,11 @@ void SetContainerOptionsFromArgs(CLIExecutionContext& context) auto const& envArgs = context.Args.GetAll(); for (const auto& arg : envArgs) { - options.EnvironmentVariables.emplace_back(WideToMultiByte(arg)); + auto envVar = EnvironmentVariable::Parse(WideToMultiByte(arg)); + if (envVar) + { + options.EnvironmentVariables.push_back(*envVar); + } } } diff --git a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp index 45b654592..f9fb9cef3 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp @@ -190,15 +190,11 @@ class WSLCE2EContainerCreateTests TEST_METHOD(WSLCE2E_Container_Run_EnvOption_KeyOnly_UsesHostValue) { - // https://github.com/microsoft/WSL/issues/14474 - SKIP_TEST_NOT_IMPL(); - WSL2_TEST_ONLY(); VerifyContainerIsNotListed(WslcContainerName); auto result = RunWslc(std::format( L"container run --rm --name {} -e {} {} env", WslcContainerName, HostEnvVariableName, DebianImage.NameAndTag())); - result.Dump(); result.Verify({.Stderr = L"", .ExitCode = S_OK}); const auto expectedLine = std::format(L"{}={}", HostEnvVariableName, HostEnvVariableValue); From de327123642b9f373b4270e2b7a8b9de4efe6314 Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Thu, 19 Mar 2026 14:30:22 -0700 Subject: [PATCH 06/17] Added UT --- src/windows/wslc/services/ContainerModel.cpp | 39 +++--- src/windows/wslc/services/ContainerModel.h | 4 +- src/windows/wslc/tasks/ContainerTasks.cpp | 11 +- .../wslc/WSLCCLIEnvVarParserUnitTests.cpp | 116 ++++++++++++++++++ 4 files changed, 149 insertions(+), 21 deletions(-) create mode 100644 test/windows/wslc/WSLCCLIEnvVarParserUnitTests.cpp diff --git a/src/windows/wslc/services/ContainerModel.cpp b/src/windows/wslc/services/ContainerModel.cpp index d4fb35323..63e0ee6cd 100644 --- a/src/windows/wslc/services/ContainerModel.cpp +++ b/src/windows/wslc/services/ContainerModel.cpp @@ -17,23 +17,23 @@ Module Name: namespace wsl::windows::wslc::models { -static inline bool IsSpace(char c) +static inline bool IsSpace(wchar_t ch) { - return std::isspace(static_cast(c)); -}; + return std::iswspace(ch) != 0; +} -std::optional EnvironmentVariable::Parse(std::string entry) +std::optional EnvironmentVariable::Parse(std::wstring entry) { if(entry.empty() || std::all_of(entry.begin(), entry.end(), IsSpace)) { return std::nullopt; } - std::string key; - std::optional value; + std::wstring key; + std::optional value; auto delimiterPos = entry.find('='); - if (delimiterPos == std::string::npos) + if (delimiterPos == std::wstring::npos) { key = entry; } @@ -55,19 +55,28 @@ std::optional EnvironmentVariable::Parse(std::string entry) if (!value.has_value()) { - const auto envValue = std::getenv(key.c_str()); - if (envValue == nullptr) + const DWORD valueLength = GetEnvironmentVariableW(key.c_str(), nullptr, 0); + if (valueLength > 0) + { + std::wstring envValue(valueLength, L'\0'); + GetEnvironmentVariableW(key.c_str(), envValue.data(), valueLength); + if (!envValue.empty() && envValue.back() == L'\0') + { + envValue.pop_back(); + } + + value = envValue; + } + else { return std::nullopt; } - - value = std::string(envValue); } - return std::format("{}={}", key, value.value()); + return std::format(L"{}={}", key, value.value()); } -std::vector EnvironmentVariable::ParseFile(std::string filePath) +std::vector EnvironmentVariable::ParseFile(std::wstring filePath) { if (!std::filesystem::exists(filePath)) { @@ -75,7 +84,7 @@ std::vector EnvironmentVariable::ParseFile(std::string filePath) } // Read the file line by line - std::vector envVars; + std::vector envVars; std::ifstream file(filePath); std::string line; while (std::getline(file, line)) @@ -91,7 +100,7 @@ std::vector EnvironmentVariable::ParseFile(std::string filePath) continue; } - auto envVar = Parse(line); + auto envVar = Parse(wsl::shared::string::MultiByteToWide(line)); if (envVar.has_value()) { envVars.push_back(std::move(envVar.value())); diff --git a/src/windows/wslc/services/ContainerModel.h b/src/windows/wslc/services/ContainerModel.h index b767b1244..720ca887a 100644 --- a/src/windows/wslc/services/ContainerModel.h +++ b/src/windows/wslc/services/ContainerModel.h @@ -70,7 +70,7 @@ struct ContainerInformation struct EnvironmentVariable { - static std::optional Parse(std::string entry); - static std::vector ParseFile(std::string filePath); + static std::optional Parse(std::wstring entry); + static std::vector ParseFile(std::wstring filePath); }; } // namespace wsl::windows::wslc::models diff --git a/src/windows/wslc/tasks/ContainerTasks.cpp b/src/windows/wslc/tasks/ContainerTasks.cpp index 77349fc2d..822e15d1c 100644 --- a/src/windows/wslc/tasks/ContainerTasks.cpp +++ b/src/windows/wslc/tasks/ContainerTasks.cpp @@ -220,8 +220,11 @@ void SetContainerOptionsFromArgs(CLIExecutionContext& context) auto const& envFiles = context.Args.GetAll(); for (const auto& envFile : envFiles) { - auto parsedEnvVars = EnvironmentVariable::ParseFile(WideToMultiByte(envFile)); - options.EnvironmentVariables.insert(options.EnvironmentVariables.end(), parsedEnvVars.begin(), parsedEnvVars.end()); + auto parsedEnvVars = EnvironmentVariable::ParseFile(envFile); + for (const auto& envVar : parsedEnvVars) + { + options.EnvironmentVariables.push_back(wsl::shared::string::WideToMultiByte(envVar)); + } } } @@ -230,10 +233,10 @@ void SetContainerOptionsFromArgs(CLIExecutionContext& context) auto const& envArgs = context.Args.GetAll(); for (const auto& arg : envArgs) { - auto envVar = EnvironmentVariable::Parse(WideToMultiByte(arg)); + auto envVar = EnvironmentVariable::Parse(arg); if (envVar) { - options.EnvironmentVariables.push_back(*envVar); + options.EnvironmentVariables.push_back(wsl::shared::string::WideToMultiByte(*envVar)); } } } diff --git a/test/windows/wslc/WSLCCLIEnvVarParserUnitTests.cpp b/test/windows/wslc/WSLCCLIEnvVarParserUnitTests.cpp new file mode 100644 index 000000000..f6ba11850 --- /dev/null +++ b/test/windows/wslc/WSLCCLIEnvVarParserUnitTests.cpp @@ -0,0 +1,116 @@ +/*++ + +Copyright (c) Microsoft. All rights reserved. + +Module Name: + + WSLCCLIEnvVarParserUnitTests.cpp + +Abstract: + + This file contains unit tests for WSLC CLI environment variable parsing and validation. + +--*/ + +#include "precomp.h" +#include "windows/Common.h" +#include "WSLCCLITestHelpers.h" +#include "ContainerModel.h" + +#include +#include + +using namespace wsl::windows::wslc; + +namespace WSLCCLIEnvVarParserUnitTests { + +class WSLCCLIEnvVarParserUnitTests +{ + WSL_TEST_CLASS(WSLCCLIEnvVarParserUnitTests) + + TEST_METHOD_SETUP(TestMethodSetup) + { + EnvTestFile = wsl::windows::common::filesystem::GetTempFilename(); + return true; + } + + TEST_METHOD_CLEANUP(TestMethodCleanup) + { + DeleteFileW(EnvTestFile.c_str()); + return true; + } + + TEST_METHOD(WSLCCLIEnvVarParser_ValidEnvVars) + { + const auto parsed = models::EnvironmentVariable::Parse(L"FOO=bar"); + VERIFY_IS_TRUE(parsed.has_value()); + VERIFY_ARE_EQUAL(L"FOO=bar", parsed.value()); + } + + TEST_METHOD(WSLCCLIEnvVarParser_UsesProcessEnvWhenValueMissing) + { + constexpr const auto key = L"WSLC_TEST_ENV_FROM_PROCESS"; + VERIFY_IS_TRUE(SetEnvironmentVariableW(key, L"process_value")); + + auto cleanup = wil::scope_exit([&] { + SetEnvironmentVariableW(key, nullptr); + }); + + const auto parsed = models::EnvironmentVariable::Parse(key); + VERIFY_IS_TRUE(parsed.has_value()); + VERIFY_ARE_EQUAL(L"WSLC_TEST_ENV_FROM_PROCESS=process_value", parsed.value()); + } + + TEST_METHOD(WSLCCLIEnvVarParser_NulloptForWhitespaceOrUnsetVar) + { + const auto whitespaceOnly = models::EnvironmentVariable::Parse(L" \t "); + VERIFY_IS_FALSE(whitespaceOnly.has_value()); + + SetEnvironmentVariableA("WSLC_TEST_ENV_UNSET", nullptr); + const auto missingFromProcess = models::EnvironmentVariable::Parse(L"WSLC_TEST_ENV_UNSET"); + VERIFY_IS_FALSE(missingFromProcess.has_value()); + } + + TEST_METHOD(WSLCCLIEnvVarParser_InvalidKeysThrow) + { + VERIFY_THROWS(models::EnvironmentVariable::Parse(L"=value"), std::exception); + VERIFY_THROWS(models::EnvironmentVariable::Parse(L"BAD KEY=value"), std::exception); + } + + TEST_METHOD(WSLCCLIEnvVarParser_ParseFileParsesAndSkipsExpectedLines) + { + constexpr const auto key = L"WSLC_TEST_ENV_FROM_FILE"; + VERIFY_IS_TRUE(SetEnvironmentVariableW(key, L"file_process_value") == TRUE); + + auto envCleanup = wil::scope_exit([&] { + SetEnvironmentVariableW(key, nullptr); + }); + + std::ofstream file(EnvTestFile); + VERIFY_IS_TRUE(file.is_open()); + file << "# comment\n"; + file << "\n"; + file << "KEY1=VALUE1\n"; + file << " KEY2=VALUE2\n"; + file << "WSLC_TEST_ENV_FROM_FILE\n"; + file << "WSLC_TEST_ENV_DOES_NOT_EXIST\n"; + file.close(); + + const auto parsed = models::EnvironmentVariable::ParseFile(EnvTestFile.wstring()); + + VERIFY_ARE_EQUAL(3, static_cast(parsed.size())); + VERIFY_ARE_EQUAL(L"KEY1=VALUE1", parsed[0]); + VERIFY_ARE_EQUAL(L"KEY2=VALUE2", parsed[1]); + VERIFY_ARE_EQUAL(L"WSLC_TEST_ENV_FROM_FILE=file_process_value", parsed[2]); + } + + TEST_METHOD(WSLCCLIEnvVarParser_ParseFileThrowsWhenMissing) + { + VERIFY_THROWS(models::EnvironmentVariable::ParseFile(L"ENV_FILE_NOT_FOUND"), std::exception); + } + +private: + std::filesystem::path EnvTestFile; +}; + +} // namespace WSLCCLIEnvVarParserUnitTests \ No newline at end of file From e7b0303999551bb934988003ad7c84d573143fb1 Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Thu, 19 Mar 2026 14:36:06 -0700 Subject: [PATCH 07/17] Added more UT --- .../wslc/WSLCCLIEnvVarParserUnitTests.cpp | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/test/windows/wslc/WSLCCLIEnvVarParserUnitTests.cpp b/test/windows/wslc/WSLCCLIEnvVarParserUnitTests.cpp index f6ba11850..5e72e3a07 100644 --- a/test/windows/wslc/WSLCCLIEnvVarParserUnitTests.cpp +++ b/test/windows/wslc/WSLCCLIEnvVarParserUnitTests.cpp @@ -75,6 +75,8 @@ class WSLCCLIEnvVarParserUnitTests { VERIFY_THROWS(models::EnvironmentVariable::Parse(L"=value"), std::exception); VERIFY_THROWS(models::EnvironmentVariable::Parse(L"BAD KEY=value"), std::exception); + VERIFY_THROWS(models::EnvironmentVariable::Parse(L"BAD\tKEY=value"), std::exception); + VERIFY_THROWS(models::EnvironmentVariable::Parse(L"BAD\nKEY=value"), std::exception); } TEST_METHOD(WSLCCLIEnvVarParser_ParseFileParsesAndSkipsExpectedLines) @@ -109,6 +111,73 @@ class WSLCCLIEnvVarParserUnitTests VERIFY_THROWS(models::EnvironmentVariable::ParseFile(L"ENV_FILE_NOT_FOUND"), std::exception); } + TEST_METHOD(WSLCCLIEnvVarParser_ExplicitEmptyValueIsValid) + { + const auto parsed = models::EnvironmentVariable::Parse(L"FOO="); + VERIFY_IS_TRUE(parsed.has_value()); + VERIFY_ARE_EQUAL(L"FOO=", parsed.value()); + } + + TEST_METHOD(WSLCCLIEnvVarParser_MultipleEqualsPreservedInValue) + { + const auto parsed = models::EnvironmentVariable::Parse(L"FOO=a=b=c"); + VERIFY_IS_TRUE(parsed.has_value()); + VERIFY_ARE_EQUAL(L"FOO=a=b=c", parsed.value()); + } + + TEST_METHOD(WSLCCLIEnvVarParser_EmptyInputReturnsNullopt) + { + const auto parsed = models::EnvironmentVariable::Parse(L""); + VERIFY_IS_FALSE(parsed.has_value()); + } + + TEST_METHOD(WSLCCLIEnvVarParser_UsesProcessEnvWhenValueIsExplicitlyEmpty) + { + constexpr const auto key = L"WSLC_TEST_ENV_EMPTY_VALUE"; + VERIFY_IS_TRUE(SetEnvironmentVariableW(key, L"")); + + auto cleanup = wil::scope_exit([&] { + SetEnvironmentVariableW(key, nullptr); + }); + + const auto parsed = models::EnvironmentVariable::Parse(key); + VERIFY_IS_TRUE(parsed.has_value()); + VERIFY_ARE_EQUAL(L"WSLC_TEST_ENV_EMPTY_VALUE=", parsed.value()); + } + + TEST_METHOD(WSLCCLIEnvVarParser_ParseFilePreservesTrailingWhitespaceInValue) + { + std::ofstream file(EnvTestFile); + VERIFY_IS_TRUE(file.is_open()); + file << "KEY=value \n"; + file.close(); + + const auto parsed = models::EnvironmentVariable::ParseFile(EnvTestFile.wstring()); + + VERIFY_ARE_EQUAL(1U, parsed.size()); + VERIFY_ARE_EQUAL(L"KEY=value ", parsed[0]); + } + + TEST_METHOD(WSLCCLIEnvVarParser_ParseFileThrowsOnInvalidLine) + { + std::ofstream file(EnvTestFile); + VERIFY_IS_TRUE(file.is_open()); + file << "BAD KEY=value\n"; + file.close(); + + VERIFY_THROWS(models::EnvironmentVariable::ParseFile(EnvTestFile.wstring()), std::exception); + } + + TEST_METHOD(WSLCCLIEnvVarParser_ParseFileEmptyFileReturnsEmpty) + { + std::ofstream file(EnvTestFile); + VERIFY_IS_TRUE(file.is_open()); + file.close(); + + const auto parsed = models::EnvironmentVariable::ParseFile(EnvTestFile.wstring()); + VERIFY_ARE_EQUAL(0U, parsed.size()); + } + private: std::filesystem::path EnvTestFile; }; From 9064bed85d9530542fbd4bf950f2af75b7ba0304 Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Thu, 19 Mar 2026 14:58:23 -0700 Subject: [PATCH 08/17] Added more E2E Tests --- .../wslc/e2e/WSLCE2EContainerCreateTests.cpp | 43 ++++++++++++++++--- 1 file changed, 38 insertions(+), 5 deletions(-) diff --git a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp index f9fb9cef3..1d29ef071 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp @@ -27,6 +27,7 @@ class WSLCE2EContainerCreateTests EnsureImageIsLoaded(DebianImage); VERIFY_IS_TRUE(::SetEnvironmentVariableW(HostEnvVariableName.c_str(), HostEnvVariableValue.c_str())); + VERIFY_IS_TRUE(::SetEnvironmentVariableW(HostEnvVariableName2.c_str(), HostEnvVariableValue2.c_str())); VERIFY_IS_TRUE(::SetEnvironmentVariableW(MissingHostEnvVariableName.c_str(), nullptr)); return true; } @@ -37,6 +38,7 @@ class WSLCE2EContainerCreateTests EnsureImageIsDeleted(DebianImage); VERIFY_IS_TRUE(::SetEnvironmentVariableW(HostEnvVariableName.c_str(), nullptr)); + VERIFY_IS_TRUE(::SetEnvironmentVariableW(HostEnvVariableName2.c_str(), nullptr)); VERIFY_IS_TRUE(::SetEnvironmentVariableW(MissingHostEnvVariableName.c_str(), nullptr)); return true; } @@ -211,27 +213,58 @@ class WSLCE2EContainerCreateTests VERIFY_IS_TRUE(foundEnv); } - TEST_METHOD(WSLCE2E_Container_Run_EnvOption_KeyOnly_HostVariableMissing) + TEST_METHOD(WSLCE2E_Container_Run_EnvOption_KeyOnly_MultipleValues_UsesHostValues) { WSL2_TEST_ONLY(); VerifyContainerIsNotListed(WslcContainerName); auto result = RunWslc(std::format( - L"container run --rm --name {} -e {} {} env", WslcContainerName, MissingHostEnvVariableName, DebianImage.NameAndTag())); + L"container run --rm --name {} -e {} -e {} {} env", + WslcContainerName, + HostEnvVariableName, + HostEnvVariableName2, + DebianImage.NameAndTag())); result.Verify({.Stderr = L"", .ExitCode = S_OK}); - const auto prefix = std::format(L"{}=", MissingHostEnvVariableName); + bool foundFirstEnv = false; + bool foundSecondEnv = false; + for (const auto& line : result.GetStdoutLines()) + { + if (line == std::format(L"{}={}", HostEnvVariableName, HostEnvVariableValue)) + { + foundFirstEnv = true; + } + else if (line == std::format(L"{}={}", HostEnvVariableName2, HostEnvVariableValue2)) + { + foundSecondEnv = true; + } + } + + VERIFY_IS_TRUE(foundFirstEnv); + VERIFY_IS_TRUE(foundSecondEnv); + } + + TEST_METHOD(WSLCE2E_Container_Run_EnvOption_EmptyValue) + { + WSL2_TEST_ONLY(); + VerifyContainerIsNotListed(WslcContainerName); + + auto result = RunWslc(std::format( + L"container run --rm --name {} -e {}= {} env", WslcContainerName, HostEnvVariableName, DebianImage.NameAndTag())); + result.Verify({.Stderr = L"", .ExitCode = S_OK}); + + const auto expectedLine = std::format(L"{}=", HostEnvVariableName); bool foundEnv = false; for (const auto& line : result.GetStdoutLines()) { - if (line.rfind(prefix, 0) == 0) + if (line == expectedLine) { foundEnv = true; break; } } - VERIFY_IS_FALSE(foundEnv); + VERIFY_IS_TRUE(foundEnv); } private: From a72c357bb4a713de78364394719eafb1a608e91a Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Thu, 19 Mar 2026 15:13:09 -0700 Subject: [PATCH 09/17] Added more E2E Tests --- .../wslc/e2e/WSLCE2EContainerCreateTests.cpp | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp index 1d29ef071..953d12b2f 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp @@ -15,6 +15,7 @@ Module Name: #include "windows/Common.h" #include "WSLCExecutor.h" #include "WSLCE2EHelpers.h" +#include namespace WSLCE2ETests { @@ -45,10 +46,19 @@ class WSLCE2EContainerCreateTests TEST_METHOD_SETUP(TestMethodSetup) { + EnvTestFile1 = wsl::windows::common::filesystem::GetTempFilename(); + EnvTestFile2 = wsl::windows::common::filesystem::GetTempFilename(); EnsureContainerDoesNotExist(WslcContainerName); return true; } + TEST_METHOD_CLEANUP(TestMethodCleanup) + { + DeleteFileW(EnvTestFile1.c_str()); + DeleteFileW(EnvTestFile2.c_str()); + return true; + } + TEST_METHOD(WSLCE2E_Container_Create_HelpCommand) { WSL2_TEST_ONLY(); @@ -267,13 +277,99 @@ class WSLCE2EContainerCreateTests VERIFY_IS_TRUE(foundEnv); } + TEST_METHOD(WSLCE2E_Container_Run_EnvFile) + { + WSL2_TEST_ONLY(); + VerifyContainerIsNotListed(WslcContainerName); + + WriteEnvFile(EnvTestFile1, { + "WSLC_TEST_ENV_FILE_A=env-file-a", + "WSLC_TEST_ENV_FILE_B=env-file-b" + }); + + auto result = RunWslc(std::format( + L"container run --rm --name {} --env-file {} {} env", + WslcContainerName, + EscapePath(EnvTestFile1.wstring()), + DebianImage.NameAndTag())); + result.Verify({.Stderr = L"", .ExitCode = S_OK}); + + bool foundFirstEnv = false; + bool foundSecondEnv = false; + for (const auto& line : result.GetStdoutLines()) + { + if (line == L"WSLC_TEST_ENV_FILE_A=env-file-a") + { + foundFirstEnv = true; + } + else if (line == L"WSLC_TEST_ENV_FILE_B=env-file-b") + { + foundSecondEnv = true; + } + } + + VERIFY_IS_TRUE(foundFirstEnv); + VERIFY_IS_TRUE(foundSecondEnv); + } + + TEST_METHOD(WSLCE2E_Container_Run_EnvOption_MixedWithEnvFile) + { + WSL2_TEST_ONLY(); + VerifyContainerIsNotListed(WslcContainerName); + + WriteEnvFile(EnvTestFile1, + { + "WSLC_TEST_ENV_MIX_FILE_A=from-file-a", + "WSLC_TEST_ENV_MIX_FILE_B=from-file-b" + }); + + auto result = RunWslc(std::format( + L"container run --rm --name {} -e WSLC_TEST_ENV_MIX_CLI=from-cli --env-file {} {} env", + WslcContainerName, + EscapePath(EnvTestFile1.wstring()), + DebianImage.NameAndTag())); + result.Verify({.Stderr = L"", .ExitCode = S_OK}); + + bool foundFileA = false; + bool foundFileB = false; + bool foundCli = false; + for (const auto& line : result.GetStdoutLines()) + { + if (line == L"WSLC_TEST_ENV_MIX_FILE_A=from-file-a") + { + foundFileA = true; + } + else if (line == L"WSLC_TEST_ENV_MIX_FILE_B=from-file-b") + { + foundFileB = true; + } + else if (line == L"WSLC_TEST_ENV_MIX_CLI=from-cli") + { + foundCli = true; + } + } + + VERIFY_IS_TRUE(foundFileA); + VERIFY_IS_TRUE(foundFileB); + VERIFY_IS_TRUE(foundCli); + } + private: + // Test container name const std::wstring WslcContainerName = L"wslc-test-container"; + + // Test environment variables const std::wstring HostEnvVariableName = L"WSLC_TEST_HOST_ENV"; const std::wstring HostEnvVariableName2 = L"WSLC_TEST_HOST_ENV2"; const std::wstring HostEnvVariableValue = L"wslc-host-env-value"; const std::wstring HostEnvVariableValue2 = L"wslc-host-env-value2"; const std::wstring MissingHostEnvVariableName = L"WSLC_TEST_MISSING_HOST_ENV"; + + // Test environment variable files + std::filesystem::path EnvTestFile1; + std::filesystem::path EnvTestFile2; + + // Test images const TestImage& DebianImage = DebianTestImage(); const TestImage& InvalidImage = InvalidTestImage(); @@ -336,5 +432,16 @@ class WSLCE2EContainerCreateTests << L" -h,--help Shows help about the selected command\r\n\r\n"; return options.str(); } + + void WriteEnvFile(const std::filesystem::path& filePath, const std::vector& envVariableLines) const + { + std::ofstream envFile(filePath, std::ios::out | std::ios::trunc | std::ios::binary); + VERIFY_IS_TRUE(envFile.is_open()); + for (const auto& line : envVariableLines) + { + envFile << line << "\n"; + } + VERIFY_IS_TRUE(envFile.good()); + }; }; } // namespace WSLCE2ETests \ No newline at end of file From 6eba41cd9916a852fab98fb720c6846b0aa50e41 Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Thu, 19 Mar 2026 15:19:47 -0700 Subject: [PATCH 10/17] Code enhancement --- .../wslc/e2e/WSLCE2EContainerCreateTests.cpp | 128 ++++-------------- 1 file changed, 27 insertions(+), 101 deletions(-) diff --git a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp index 953d12b2f..a96eecca8 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp @@ -156,17 +156,7 @@ class WSLCE2EContainerCreateTests result.Verify({.Stderr = L"", .ExitCode = S_OK}); const auto outputLines = result.GetStdoutLines(); - bool foundEnv = false; - for (const auto& line : outputLines) - { - if (line == std::format(L"{}=A", HostEnvVariableName)) - { - foundEnv = true; - break; - } - } - - VERIFY_IS_TRUE(foundEnv); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, std::format(L"{}=A", HostEnvVariableName))); } TEST_METHOD(WSLCE2E_Container_Run_EnvOption_MultipleValues) @@ -182,22 +172,9 @@ class WSLCE2EContainerCreateTests DebianImage.NameAndTag())); result.Verify({.Stderr = L"", .ExitCode = S_OK}); - bool foundFirstEnv = false; - bool foundSecondEnv = false; - for (const auto& line : result.GetStdoutLines()) - { - if (line == std::format(L"{}=A", HostEnvVariableName)) - { - foundFirstEnv = true; - } - else if (line == std::format(L"{}=B", HostEnvVariableName2)) - { - foundSecondEnv = true; - } - } - - VERIFY_IS_TRUE(foundFirstEnv); - VERIFY_IS_TRUE(foundSecondEnv); + const auto outputLines = result.GetStdoutLines(); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, std::format(L"{}=A", HostEnvVariableName))); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, std::format(L"{}=B", HostEnvVariableName2))); } TEST_METHOD(WSLCE2E_Container_Run_EnvOption_KeyOnly_UsesHostValue) @@ -209,18 +186,8 @@ class WSLCE2EContainerCreateTests L"container run --rm --name {} -e {} {} env", WslcContainerName, HostEnvVariableName, DebianImage.NameAndTag())); result.Verify({.Stderr = L"", .ExitCode = S_OK}); - const auto expectedLine = std::format(L"{}={}", HostEnvVariableName, HostEnvVariableValue); - bool foundEnv = false; - for (const auto& line : result.GetStdoutLines()) - { - if (line == expectedLine) - { - foundEnv = true; - break; - } - } - - VERIFY_IS_TRUE(foundEnv); + const auto outputLines = result.GetStdoutLines(); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, std::format(L"{}={}", HostEnvVariableName, HostEnvVariableValue))); } TEST_METHOD(WSLCE2E_Container_Run_EnvOption_KeyOnly_MultipleValues_UsesHostValues) @@ -236,22 +203,9 @@ class WSLCE2EContainerCreateTests DebianImage.NameAndTag())); result.Verify({.Stderr = L"", .ExitCode = S_OK}); - bool foundFirstEnv = false; - bool foundSecondEnv = false; - for (const auto& line : result.GetStdoutLines()) - { - if (line == std::format(L"{}={}", HostEnvVariableName, HostEnvVariableValue)) - { - foundFirstEnv = true; - } - else if (line == std::format(L"{}={}", HostEnvVariableName2, HostEnvVariableValue2)) - { - foundSecondEnv = true; - } - } - - VERIFY_IS_TRUE(foundFirstEnv); - VERIFY_IS_TRUE(foundSecondEnv); + const auto outputLines = result.GetStdoutLines(); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, std::format(L"{}={}", HostEnvVariableName, HostEnvVariableValue))); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, std::format(L"{}={}", HostEnvVariableName2, HostEnvVariableValue2))); } TEST_METHOD(WSLCE2E_Container_Run_EnvOption_EmptyValue) @@ -263,18 +217,8 @@ class WSLCE2EContainerCreateTests L"container run --rm --name {} -e {}= {} env", WslcContainerName, HostEnvVariableName, DebianImage.NameAndTag())); result.Verify({.Stderr = L"", .ExitCode = S_OK}); - const auto expectedLine = std::format(L"{}=", HostEnvVariableName); - bool foundEnv = false; - for (const auto& line : result.GetStdoutLines()) - { - if (line == expectedLine) - { - foundEnv = true; - break; - } - } - - VERIFY_IS_TRUE(foundEnv); + const auto outputLines = result.GetStdoutLines(); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, std::format(L"{}=", HostEnvVariableName))); } TEST_METHOD(WSLCE2E_Container_Run_EnvFile) @@ -294,22 +238,9 @@ class WSLCE2EContainerCreateTests DebianImage.NameAndTag())); result.Verify({.Stderr = L"", .ExitCode = S_OK}); - bool foundFirstEnv = false; - bool foundSecondEnv = false; - for (const auto& line : result.GetStdoutLines()) - { - if (line == L"WSLC_TEST_ENV_FILE_A=env-file-a") - { - foundFirstEnv = true; - } - else if (line == L"WSLC_TEST_ENV_FILE_B=env-file-b") - { - foundSecondEnv = true; - } - } - - VERIFY_IS_TRUE(foundFirstEnv); - VERIFY_IS_TRUE(foundSecondEnv); + const auto outputLines = result.GetStdoutLines(); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, L"WSLC_TEST_ENV_FILE_A=env-file-a")); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, L"WSLC_TEST_ENV_FILE_B=env-file-b")); } TEST_METHOD(WSLCE2E_Container_Run_EnvOption_MixedWithEnvFile) @@ -330,31 +261,26 @@ class WSLCE2EContainerCreateTests DebianImage.NameAndTag())); result.Verify({.Stderr = L"", .ExitCode = S_OK}); - bool foundFileA = false; - bool foundFileB = false; - bool foundCli = false; - for (const auto& line : result.GetStdoutLines()) + const auto outputLines = result.GetStdoutLines(); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, L"WSLC_TEST_ENV_MIX_FILE_A=from-file-a")); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, L"WSLC_TEST_ENV_MIX_FILE_B=from-file-b")); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, L"WSLC_TEST_ENV_MIX_CLI=from-cli")); + } + +private: + bool ContainsOutputLine(const std::vector& outputLines, const std::wstring& expectedLine) const + { + for (const auto& line : outputLines) { - if (line == L"WSLC_TEST_ENV_MIX_FILE_A=from-file-a") - { - foundFileA = true; - } - else if (line == L"WSLC_TEST_ENV_MIX_FILE_B=from-file-b") - { - foundFileB = true; - } - else if (line == L"WSLC_TEST_ENV_MIX_CLI=from-cli") + if (line == expectedLine) { - foundCli = true; + return true; } } - VERIFY_IS_TRUE(foundFileA); - VERIFY_IS_TRUE(foundFileB); - VERIFY_IS_TRUE(foundCli); + return false; } -private: // Test container name const std::wstring WslcContainerName = L"wslc-test-container"; From a8aefdddf719bb33f54df914bfee18e783920643 Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Thu, 19 Mar 2026 15:23:05 -0700 Subject: [PATCH 11/17] Added more E2E Tests --- .../wslc/e2e/WSLCE2EContainerCreateTests.cpp | 50 +++++++++++++++---- 1 file changed, 40 insertions(+), 10 deletions(-) diff --git a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp index a96eecca8..b8dbe324f 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp @@ -267,20 +267,37 @@ class WSLCE2EContainerCreateTests VERIFY_IS_TRUE(ContainsOutputLine(outputLines, L"WSLC_TEST_ENV_MIX_CLI=from-cli")); } -private: - bool ContainsOutputLine(const std::vector& outputLines, const std::wstring& expectedLine) const + TEST_METHOD(WSLCE2E_Container_Run_EnvFile_MultipleFiles) { - for (const auto& line : outputLines) - { - if (line == expectedLine) - { - return true; - } - } + WSL2_TEST_ONLY(); + VerifyContainerIsNotListed(WslcContainerName); - return false; + WriteEnvFile(EnvTestFile1, { + "WSLC_TEST_ENV_FILE_MULTI_A=file1-a", + "WSLC_TEST_ENV_FILE_MULTI_B=file1-b" + }); + + WriteEnvFile(EnvTestFile2, { + "WSLC_TEST_ENV_FILE_MULTI_C=file2-c", + "WSLC_TEST_ENV_FILE_MULTI_D=file2-d" + }); + + auto result = RunWslc(std::format( + L"container run --rm --name {} --env-file {} --env-file {} {} env", + WslcContainerName, + EscapePath(EnvTestFile1.wstring()), + EscapePath(EnvTestFile2.wstring()), + DebianImage.NameAndTag())); + result.Verify({.Stderr = L"", .ExitCode = S_OK}); + + const auto outputLines = result.GetStdoutLines(); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, L"WSLC_TEST_ENV_FILE_MULTI_A=file1-a")); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, L"WSLC_TEST_ENV_FILE_MULTI_B=file1-b")); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, L"WSLC_TEST_ENV_FILE_MULTI_C=file2-c")); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, L"WSLC_TEST_ENV_FILE_MULTI_D=file2-d")); } +private: // Test container name const std::wstring WslcContainerName = L"wslc-test-container"; @@ -369,5 +386,18 @@ class WSLCE2EContainerCreateTests } VERIFY_IS_TRUE(envFile.good()); }; + + bool ContainsOutputLine(const std::vector& outputLines, const std::wstring& expectedLine) const + { + for (const auto& line : outputLines) + { + if (line == expectedLine) + { + return true; + } + } + + return false; + } }; } // namespace WSLCE2ETests \ No newline at end of file From 63817932ec84750b49e78697d38ff91c89c5b5ab Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Thu, 19 Mar 2026 15:47:48 -0700 Subject: [PATCH 12/17] Added more E2E Tests --- src/windows/wslc/services/ContainerModel.cpp | 2 +- .../wslc/e2e/WSLCE2EContainerCreateTests.cpp | 87 +++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/src/windows/wslc/services/ContainerModel.cpp b/src/windows/wslc/services/ContainerModel.cpp index 63e0ee6cd..8c87b36d2 100644 --- a/src/windows/wslc/services/ContainerModel.cpp +++ b/src/windows/wslc/services/ContainerModel.cpp @@ -80,7 +80,7 @@ std::vector EnvironmentVariable::ParseFile(std::wstring filePath) { if (!std::filesystem::exists(filePath)) { - throw std::invalid_argument(std::format("Environment file '{}' does not exist", filePath)); + THROW_HR_WITH_USER_ERROR(E_INVALIDARG, std::format(L"Environment file '{}' does not exist", filePath)); } // Read the file line by line diff --git a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp index b8dbe324f..272ee0e20 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp @@ -297,6 +297,93 @@ class WSLCE2EContainerCreateTests VERIFY_IS_TRUE(ContainsOutputLine(outputLines, L"WSLC_TEST_ENV_FILE_MULTI_D=file2-d")); } + TEST_METHOD(WSLCE2E_Container_Run_EnvFile_MissingFile) + { + WSL2_TEST_ONLY(); + VerifyContainerIsNotListed(WslcContainerName); + + auto result = RunWslc(std::format( + L"container run --rm --name {} --env-file ENV_FILE_NOT_FOUND {} env", + WslcContainerName, + DebianImage.NameAndTag())); + result.Verify({.Stderr = L"Environment file 'ENV_FILE_NOT_FOUND' does not exist\r\nError code: E_INVALIDARG\r\n", .ExitCode = 1}); + } + + TEST_METHOD(WSLCE2E_Container_Run_EnvFile_InvalidContent) + { + WSL2_TEST_ONLY(); + VerifyContainerIsNotListed(WslcContainerName); + + WriteEnvFile(EnvTestFile1, { + "WSLC_TEST_ENV_VALID=ok", + "BAD KEY=value" + }); + + auto result = RunWslc(std::format( + L"container run --rm --name {} --env-file {} {} env", + WslcContainerName, + EscapePath(EnvTestFile1.wstring()), + DebianImage.NameAndTag())); + result.Verify({.Stderr = L"Environment variable key 'BAD KEY' cannot contain whitespace\r\nError code: E_INVALIDARG\r\n", .ExitCode = 1}); + } + + TEST_METHOD(WSLCE2E_Container_Run_EnvFile_DuplicateKeys_Precedence) + { + WSL2_TEST_ONLY(); + VerifyContainerIsNotListed(WslcContainerName); + + WriteEnvFile(EnvTestFile1, { + "WSLC_TEST_ENV_DUP=from-file-1" + }); + + WriteEnvFile(EnvTestFile2, { + "WSLC_TEST_ENV_DUP=from-file-2" + }); + + // Later --env-file should win over earlier --env-file for duplicate keys + auto result = RunWslc(std::format( + L"container run --rm --name {} --env-file {} --env-file {} {} env", + WslcContainerName, + EscapePath(EnvTestFile1.wstring()), + EscapePath(EnvTestFile2.wstring()), + DebianImage.NameAndTag())); + result.Verify({.Stderr = L"", .ExitCode = S_OK}); + + auto outputLines = result.GetStdoutLines(); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, L"WSLC_TEST_ENV_DUP=from-file-2")); + + // Explicit -e should win over env-file value for duplicate keys + result = RunWslc(std::format( + L"container run --rm --name {} -e WSLC_TEST_ENV_DUP=from-cli --env-file {} --env-file {} {} env", + WslcContainerName, + EscapePath(EnvTestFile1.wstring()), + EscapePath(EnvTestFile2.wstring()), + DebianImage.NameAndTag())); + result.Verify({.Stderr = L"", .ExitCode = S_OK}); + + outputLines = result.GetStdoutLines(); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, L"WSLC_TEST_ENV_DUP=from-cli")); + } + + TEST_METHOD(WSLCE2E_Container_Run_EnvFile_ValueContainsEquals) + { + WSL2_TEST_ONLY(); + VerifyContainerIsNotListed(WslcContainerName); + + WriteEnvFile(EnvTestFile1, { + "WSLC_TEST_ENV_EQUALS=value=with=equals" + }); + + auto result = RunWslc(std::format( + L"container run --rm --name {} --env-file {} {} env", + WslcContainerName, + EscapePath(EnvTestFile1.wstring()), + DebianImage.NameAndTag())); + result.Verify({.Stderr = L"", .ExitCode = S_OK}); + + const auto outputLines = result.GetStdoutLines(); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, L"WSLC_TEST_ENV_EQUALS=value=with=equals")); + } private: // Test container name const std::wstring WslcContainerName = L"wslc-test-container"; From 6af19b1e682d511be5e4e51ccf66bce6044438d6 Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Thu, 19 Mar 2026 22:52:17 -0700 Subject: [PATCH 13/17] Added more E2E Tests --- .../wslc/commands/ContainerExecCommand.cpp | 4 +- .../wslc/services/ContainerService.cpp | 2 +- .../wslc/e2e/WSLCE2EContainerCreateTests.cpp | 104 ++++++++++++++++++ test/windows/wslc/e2e/WSLCE2EHelpers.cpp | 6 + 4 files changed, 113 insertions(+), 3 deletions(-) diff --git a/src/windows/wslc/commands/ContainerExecCommand.cpp b/src/windows/wslc/commands/ContainerExecCommand.cpp index 368307b72..999d44891 100644 --- a/src/windows/wslc/commands/ContainerExecCommand.cpp +++ b/src/windows/wslc/commands/ContainerExecCommand.cpp @@ -34,8 +34,8 @@ std::vector ContainerExecCommand::GetArguments() const std::nullopt, L"Arguments to pass to the command being executed inside the container"), Argument::Create(ArgType::Detach), - Argument::Create(ArgType::Env, std::nullopt, NO_LIMIT), - Argument::Create(ArgType::EnvFile), + Argument::Create(ArgType::Env, false, NO_LIMIT), + Argument::Create(ArgType::EnvFile, false, NO_LIMIT), Argument::Create(ArgType::Interactive), Argument::Create(ArgType::Session), Argument::Create(ArgType::TTY), diff --git a/src/windows/wslc/services/ContainerService.cpp b/src/windows/wslc/services/ContainerService.cpp index b4f286d8b..0b569f689 100644 --- a/src/windows/wslc/services/ContainerService.cpp +++ b/src/windows/wslc/services/ContainerService.cpp @@ -299,7 +299,7 @@ int ContainerService::Exec(Session& session, const std::string& id, ContainerOpt ConsoleService consoleService; return consoleService.AttachToCurrentConsole( - wsl::windows::common::WSLAProcessLauncher({}, options.Arguments, {}, execFlags).Launch(*container)); + wsl::windows::common::WSLAProcessLauncher({}, options.Arguments, options.EnvironmentVariables, execFlags).Launch(*container)); } InspectContainer ContainerService::Inspect(Session& session, const std::string& id) diff --git a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp index 272ee0e20..975d69cfe 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp @@ -384,6 +384,110 @@ class WSLCE2EContainerCreateTests const auto outputLines = result.GetStdoutLines(); VERIFY_IS_TRUE(ContainsOutputLine(outputLines, L"WSLC_TEST_ENV_EQUALS=value=with=equals")); } + + TEST_METHOD(WSLCE2E_Container_Exec_EnvOption) + { + WSL2_TEST_ONLY(); + + auto result = RunWslc(std::format( + L"container run -d --name {} {} sleep infinity", + WslcContainerName, DebianImage.NameAndTag())); + result.Verify({.Stderr = L"", .ExitCode = S_OK}); + + result = RunWslc(std::format( + L"container exec -e {}=A {} env", + HostEnvVariableName, + WslcContainerName)); + result.Verify({.Stderr = L"", .ExitCode = S_OK}); + + const auto outputLines = result.GetStdoutLines(); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, std::format(L"{}=A", HostEnvVariableName))); + } + + TEST_METHOD(WSLCE2E_Container_Exec_EnvOption_KeyOnly_UsesHostValue) + { + WSL2_TEST_ONLY(); + + auto result = RunWslc(std::format( + L"container run -d --name {} {} sleep infinity", + WslcContainerName, DebianImage.NameAndTag())); + result.Verify({.Stderr = L"", .ExitCode = S_OK}); + + result = RunWslc(std::format( + L"container exec -e {} {} env", + HostEnvVariableName, + WslcContainerName)); + result.Verify({.Stderr = L"", .ExitCode = S_OK}); + + const auto outputLines = result.GetStdoutLines(); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, std::format(L"{}={}", HostEnvVariableName, HostEnvVariableValue))); + } + + TEST_METHOD(WSLCE2E_Container_Exec_EnvFile) + { + WSL2_TEST_ONLY(); + + WriteEnvFile(EnvTestFile1, { + "WSLC_TEST_EXEC_ENV_FILE_A=exec-env-file-a", + "WSLC_TEST_EXEC_ENV_FILE_B=exec-env-file-b" + }); + + auto result = RunWslc(std::format( + L"container run -d --name {} {} sleep infinity", + WslcContainerName, DebianImage.NameAndTag())); + result.Verify({.Stderr = L"", .ExitCode = S_OK}); + + result = RunWslc(std::format( + L"container exec --env-file {} {} env", + EscapePath(EnvTestFile1.wstring()), + WslcContainerName)); + result.Verify({.Stderr = L"", .ExitCode = S_OK}); + + const auto outputLines = result.GetStdoutLines(); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, L"WSLC_TEST_EXEC_ENV_FILE_A=exec-env-file-a")); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, L"WSLC_TEST_EXEC_ENV_FILE_B=exec-env-file-b")); + } + + TEST_METHOD(WSLCE2E_Container_Exec_EnvOption_MixedWithEnvFile) + { + WSL2_TEST_ONLY(); + + WriteEnvFile(EnvTestFile1, { + "WSLC_TEST_EXEC_ENV_MIX_FILE_A=from-file-a", + "WSLC_TEST_EXEC_ENV_MIX_FILE_B=from-file-b" + }); + + auto result = RunWslc(std::format( + L"container run -d --name {} {} sleep infinity", + WslcContainerName, DebianImage.NameAndTag())); + result.Verify({.Stderr = L"", .ExitCode = S_OK}); + + result = RunWslc(std::format( + L"container exec -e WSLC_TEST_EXEC_ENV_MIX_CLI=from-cli --env-file {} {} env", + EscapePath(EnvTestFile1.wstring()), + WslcContainerName)); + result.Verify({.Stderr = L"", .ExitCode = S_OK}); + + const auto outputLines = result.GetStdoutLines(); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, L"WSLC_TEST_EXEC_ENV_MIX_FILE_A=from-file-a")); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, L"WSLC_TEST_EXEC_ENV_MIX_FILE_B=from-file-b")); + VERIFY_IS_TRUE(ContainsOutputLine(outputLines, L"WSLC_TEST_EXEC_ENV_MIX_CLI=from-cli")); + } + + TEST_METHOD(WSLCE2E_Container_Exec_EnvFile_MissingFile) + { + WSL2_TEST_ONLY(); + + auto result = RunWslc(std::format( + L"container run -d --name {} {} sleep infinity", + WslcContainerName, DebianImage.NameAndTag())); + result.Verify({.Stderr = L"", .ExitCode = S_OK}); + + result = RunWslc(std::format( + L"container exec --env-file ENV_FILE_NOT_FOUND {} env", + WslcContainerName)); + result.Verify({.Stderr = L"Environment file 'ENV_FILE_NOT_FOUND' does not exist\r\nError code: E_INVALIDARG\r\n", .ExitCode = 1}); + } private: // Test container name const std::wstring WslcContainerName = L"wslc-test-container"; diff --git a/test/windows/wslc/e2e/WSLCE2EHelpers.cpp b/test/windows/wslc/e2e/WSLCE2EHelpers.cpp index 5c492b347..1b8c47dbc 100644 --- a/test/windows/wslc/e2e/WSLCE2EHelpers.cpp +++ b/test/windows/wslc/e2e/WSLCE2EHelpers.cpp @@ -136,6 +136,12 @@ void EnsureContainerDoesNotExist(const std::wstring& containerName) { if (line.find(containerName) != std::wstring::npos) { + if (line.find(L"running") != std::wstring::npos) + { + auto result = RunWslc(std::format(L"container kill {}", containerName)); + result.Verify({.Stdout = L"", .Stderr = L"", .ExitCode = 0}); + } + auto result = RunWslc(std::format(L"container remove {}", containerName)); result.Verify({.Stderr = L"", .ExitCode = 0}); break; From 289c0316ef547ca4334767280a78b12a8eed64ac Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Thu, 19 Mar 2026 22:52:49 -0700 Subject: [PATCH 14/17] Clang format --- src/windows/wslc/services/ContainerModel.cpp | 6 +- .../wslc/WSLCCLIEnvVarParserUnitTests.cpp | 12 +-- .../wslc/e2e/WSLCE2EContainerCreateTests.cpp | 96 +++++-------------- 3 files changed, 27 insertions(+), 87 deletions(-) diff --git a/src/windows/wslc/services/ContainerModel.cpp b/src/windows/wslc/services/ContainerModel.cpp index 8c87b36d2..d20254b09 100644 --- a/src/windows/wslc/services/ContainerModel.cpp +++ b/src/windows/wslc/services/ContainerModel.cpp @@ -24,7 +24,7 @@ static inline bool IsSpace(wchar_t ch) std::optional EnvironmentVariable::Parse(std::wstring entry) { - if(entry.empty() || std::all_of(entry.begin(), entry.end(), IsSpace)) + if (entry.empty() || std::all_of(entry.begin(), entry.end(), IsSpace)) { return std::nullopt; } @@ -90,9 +90,7 @@ std::vector EnvironmentVariable::ParseFile(std::wstring filePath) while (std::getline(file, line)) { // Remove leading whitespace - line.erase(line.begin(), std::find_if(line.begin(), line.end(), [](unsigned char ch) { - return !std::isspace(ch); - })); + line.erase(line.begin(), std::find_if(line.begin(), line.end(), [](unsigned char ch) { return !std::isspace(ch); })); // Skip empty lines and comments if (line.empty() || line[0] == '#') diff --git a/test/windows/wslc/WSLCCLIEnvVarParserUnitTests.cpp b/test/windows/wslc/WSLCCLIEnvVarParserUnitTests.cpp index 5e72e3a07..1f0ef2f18 100644 --- a/test/windows/wslc/WSLCCLIEnvVarParserUnitTests.cpp +++ b/test/windows/wslc/WSLCCLIEnvVarParserUnitTests.cpp @@ -52,9 +52,7 @@ class WSLCCLIEnvVarParserUnitTests constexpr const auto key = L"WSLC_TEST_ENV_FROM_PROCESS"; VERIFY_IS_TRUE(SetEnvironmentVariableW(key, L"process_value")); - auto cleanup = wil::scope_exit([&] { - SetEnvironmentVariableW(key, nullptr); - }); + auto cleanup = wil::scope_exit([&] { SetEnvironmentVariableW(key, nullptr); }); const auto parsed = models::EnvironmentVariable::Parse(key); VERIFY_IS_TRUE(parsed.has_value()); @@ -84,9 +82,7 @@ class WSLCCLIEnvVarParserUnitTests constexpr const auto key = L"WSLC_TEST_ENV_FROM_FILE"; VERIFY_IS_TRUE(SetEnvironmentVariableW(key, L"file_process_value") == TRUE); - auto envCleanup = wil::scope_exit([&] { - SetEnvironmentVariableW(key, nullptr); - }); + auto envCleanup = wil::scope_exit([&] { SetEnvironmentVariableW(key, nullptr); }); std::ofstream file(EnvTestFile); VERIFY_IS_TRUE(file.is_open()); @@ -136,9 +132,7 @@ class WSLCCLIEnvVarParserUnitTests constexpr const auto key = L"WSLC_TEST_ENV_EMPTY_VALUE"; VERIFY_IS_TRUE(SetEnvironmentVariableW(key, L"")); - auto cleanup = wil::scope_exit([&] { - SetEnvironmentVariableW(key, nullptr); - }); + auto cleanup = wil::scope_exit([&] { SetEnvironmentVariableW(key, nullptr); }); const auto parsed = models::EnvironmentVariable::Parse(key); VERIFY_IS_TRUE(parsed.has_value()); diff --git a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp index 975d69cfe..e8eeb80c9 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp @@ -226,10 +226,7 @@ class WSLCE2EContainerCreateTests WSL2_TEST_ONLY(); VerifyContainerIsNotListed(WslcContainerName); - WriteEnvFile(EnvTestFile1, { - "WSLC_TEST_ENV_FILE_A=env-file-a", - "WSLC_TEST_ENV_FILE_B=env-file-b" - }); + WriteEnvFile(EnvTestFile1, {"WSLC_TEST_ENV_FILE_A=env-file-a", "WSLC_TEST_ENV_FILE_B=env-file-b"}); auto result = RunWslc(std::format( L"container run --rm --name {} --env-file {} {} env", @@ -248,11 +245,7 @@ class WSLCE2EContainerCreateTests WSL2_TEST_ONLY(); VerifyContainerIsNotListed(WslcContainerName); - WriteEnvFile(EnvTestFile1, - { - "WSLC_TEST_ENV_MIX_FILE_A=from-file-a", - "WSLC_TEST_ENV_MIX_FILE_B=from-file-b" - }); + WriteEnvFile(EnvTestFile1, {"WSLC_TEST_ENV_MIX_FILE_A=from-file-a", "WSLC_TEST_ENV_MIX_FILE_B=from-file-b"}); auto result = RunWslc(std::format( L"container run --rm --name {} -e WSLC_TEST_ENV_MIX_CLI=from-cli --env-file {} {} env", @@ -272,15 +265,9 @@ class WSLCE2EContainerCreateTests WSL2_TEST_ONLY(); VerifyContainerIsNotListed(WslcContainerName); - WriteEnvFile(EnvTestFile1, { - "WSLC_TEST_ENV_FILE_MULTI_A=file1-a", - "WSLC_TEST_ENV_FILE_MULTI_B=file1-b" - }); + WriteEnvFile(EnvTestFile1, {"WSLC_TEST_ENV_FILE_MULTI_A=file1-a", "WSLC_TEST_ENV_FILE_MULTI_B=file1-b"}); - WriteEnvFile(EnvTestFile2, { - "WSLC_TEST_ENV_FILE_MULTI_C=file2-c", - "WSLC_TEST_ENV_FILE_MULTI_D=file2-d" - }); + WriteEnvFile(EnvTestFile2, {"WSLC_TEST_ENV_FILE_MULTI_C=file2-c", "WSLC_TEST_ENV_FILE_MULTI_D=file2-d"}); auto result = RunWslc(std::format( L"container run --rm --name {} --env-file {} --env-file {} {} env", @@ -303,9 +290,7 @@ class WSLCE2EContainerCreateTests VerifyContainerIsNotListed(WslcContainerName); auto result = RunWslc(std::format( - L"container run --rm --name {} --env-file ENV_FILE_NOT_FOUND {} env", - WslcContainerName, - DebianImage.NameAndTag())); + L"container run --rm --name {} --env-file ENV_FILE_NOT_FOUND {} env", WslcContainerName, DebianImage.NameAndTag())); result.Verify({.Stderr = L"Environment file 'ENV_FILE_NOT_FOUND' does not exist\r\nError code: E_INVALIDARG\r\n", .ExitCode = 1}); } @@ -314,10 +299,7 @@ class WSLCE2EContainerCreateTests WSL2_TEST_ONLY(); VerifyContainerIsNotListed(WslcContainerName); - WriteEnvFile(EnvTestFile1, { - "WSLC_TEST_ENV_VALID=ok", - "BAD KEY=value" - }); + WriteEnvFile(EnvTestFile1, {"WSLC_TEST_ENV_VALID=ok", "BAD KEY=value"}); auto result = RunWslc(std::format( L"container run --rm --name {} --env-file {} {} env", @@ -332,13 +314,9 @@ class WSLCE2EContainerCreateTests WSL2_TEST_ONLY(); VerifyContainerIsNotListed(WslcContainerName); - WriteEnvFile(EnvTestFile1, { - "WSLC_TEST_ENV_DUP=from-file-1" - }); + WriteEnvFile(EnvTestFile1, {"WSLC_TEST_ENV_DUP=from-file-1"}); - WriteEnvFile(EnvTestFile2, { - "WSLC_TEST_ENV_DUP=from-file-2" - }); + WriteEnvFile(EnvTestFile2, {"WSLC_TEST_ENV_DUP=from-file-2"}); // Later --env-file should win over earlier --env-file for duplicate keys auto result = RunWslc(std::format( @@ -370,9 +348,7 @@ class WSLCE2EContainerCreateTests WSL2_TEST_ONLY(); VerifyContainerIsNotListed(WslcContainerName); - WriteEnvFile(EnvTestFile1, { - "WSLC_TEST_ENV_EQUALS=value=with=equals" - }); + WriteEnvFile(EnvTestFile1, {"WSLC_TEST_ENV_EQUALS=value=with=equals"}); auto result = RunWslc(std::format( L"container run --rm --name {} --env-file {} {} env", @@ -389,15 +365,10 @@ class WSLCE2EContainerCreateTests { WSL2_TEST_ONLY(); - auto result = RunWslc(std::format( - L"container run -d --name {} {} sleep infinity", - WslcContainerName, DebianImage.NameAndTag())); + auto result = RunWslc(std::format(L"container run -d --name {} {} sleep infinity", WslcContainerName, DebianImage.NameAndTag())); result.Verify({.Stderr = L"", .ExitCode = S_OK}); - result = RunWslc(std::format( - L"container exec -e {}=A {} env", - HostEnvVariableName, - WslcContainerName)); + result = RunWslc(std::format(L"container exec -e {}=A {} env", HostEnvVariableName, WslcContainerName)); result.Verify({.Stderr = L"", .ExitCode = S_OK}); const auto outputLines = result.GetStdoutLines(); @@ -408,15 +379,10 @@ class WSLCE2EContainerCreateTests { WSL2_TEST_ONLY(); - auto result = RunWslc(std::format( - L"container run -d --name {} {} sleep infinity", - WslcContainerName, DebianImage.NameAndTag())); + auto result = RunWslc(std::format(L"container run -d --name {} {} sleep infinity", WslcContainerName, DebianImage.NameAndTag())); result.Verify({.Stderr = L"", .ExitCode = S_OK}); - result = RunWslc(std::format( - L"container exec -e {} {} env", - HostEnvVariableName, - WslcContainerName)); + result = RunWslc(std::format(L"container exec -e {} {} env", HostEnvVariableName, WslcContainerName)); result.Verify({.Stderr = L"", .ExitCode = S_OK}); const auto outputLines = result.GetStdoutLines(); @@ -427,20 +393,12 @@ class WSLCE2EContainerCreateTests { WSL2_TEST_ONLY(); - WriteEnvFile(EnvTestFile1, { - "WSLC_TEST_EXEC_ENV_FILE_A=exec-env-file-a", - "WSLC_TEST_EXEC_ENV_FILE_B=exec-env-file-b" - }); + WriteEnvFile(EnvTestFile1, {"WSLC_TEST_EXEC_ENV_FILE_A=exec-env-file-a", "WSLC_TEST_EXEC_ENV_FILE_B=exec-env-file-b"}); - auto result = RunWslc(std::format( - L"container run -d --name {} {} sleep infinity", - WslcContainerName, DebianImage.NameAndTag())); + auto result = RunWslc(std::format(L"container run -d --name {} {} sleep infinity", WslcContainerName, DebianImage.NameAndTag())); result.Verify({.Stderr = L"", .ExitCode = S_OK}); - result = RunWslc(std::format( - L"container exec --env-file {} {} env", - EscapePath(EnvTestFile1.wstring()), - WslcContainerName)); + result = RunWslc(std::format(L"container exec --env-file {} {} env", EscapePath(EnvTestFile1.wstring()), WslcContainerName)); result.Verify({.Stderr = L"", .ExitCode = S_OK}); const auto outputLines = result.GetStdoutLines(); @@ -452,20 +410,13 @@ class WSLCE2EContainerCreateTests { WSL2_TEST_ONLY(); - WriteEnvFile(EnvTestFile1, { - "WSLC_TEST_EXEC_ENV_MIX_FILE_A=from-file-a", - "WSLC_TEST_EXEC_ENV_MIX_FILE_B=from-file-b" - }); + WriteEnvFile(EnvTestFile1, {"WSLC_TEST_EXEC_ENV_MIX_FILE_A=from-file-a", "WSLC_TEST_EXEC_ENV_MIX_FILE_B=from-file-b"}); - auto result = RunWslc(std::format( - L"container run -d --name {} {} sleep infinity", - WslcContainerName, DebianImage.NameAndTag())); + auto result = RunWslc(std::format(L"container run -d --name {} {} sleep infinity", WslcContainerName, DebianImage.NameAndTag())); result.Verify({.Stderr = L"", .ExitCode = S_OK}); result = RunWslc(std::format( - L"container exec -e WSLC_TEST_EXEC_ENV_MIX_CLI=from-cli --env-file {} {} env", - EscapePath(EnvTestFile1.wstring()), - WslcContainerName)); + L"container exec -e WSLC_TEST_EXEC_ENV_MIX_CLI=from-cli --env-file {} {} env", EscapePath(EnvTestFile1.wstring()), WslcContainerName)); result.Verify({.Stderr = L"", .ExitCode = S_OK}); const auto outputLines = result.GetStdoutLines(); @@ -478,16 +429,13 @@ class WSLCE2EContainerCreateTests { WSL2_TEST_ONLY(); - auto result = RunWslc(std::format( - L"container run -d --name {} {} sleep infinity", - WslcContainerName, DebianImage.NameAndTag())); + auto result = RunWslc(std::format(L"container run -d --name {} {} sleep infinity", WslcContainerName, DebianImage.NameAndTag())); result.Verify({.Stderr = L"", .ExitCode = S_OK}); - result = RunWslc(std::format( - L"container exec --env-file ENV_FILE_NOT_FOUND {} env", - WslcContainerName)); + result = RunWslc(std::format(L"container exec --env-file ENV_FILE_NOT_FOUND {} env", WslcContainerName)); result.Verify({.Stderr = L"Environment file 'ENV_FILE_NOT_FOUND' does not exist\r\nError code: E_INVALIDARG\r\n", .ExitCode = 1}); } + private: // Test container name const std::wstring WslcContainerName = L"wslc-test-container"; From 5e8904b1c9331a0f2aecbada6433c400bd4293c0 Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Fri, 20 Mar 2026 14:05:25 -0700 Subject: [PATCH 15/17] Resolve copilot comment --- src/windows/wslc/services/ContainerModel.cpp | 10 +++++++--- test/windows/wslc/WSLCCLIEnvVarParserUnitTests.cpp | 2 +- test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/windows/wslc/services/ContainerModel.cpp b/src/windows/wslc/services/ContainerModel.cpp index 76863c132..5a8ef50a2 100644 --- a/src/windows/wslc/services/ContainerModel.cpp +++ b/src/windows/wslc/services/ContainerModel.cpp @@ -13,7 +13,6 @@ Module Name: #include "precomp.h" #include "ContainerModel.h" -#include namespace wsl::windows::wslc::models { @@ -88,7 +87,7 @@ std::optional EnvironmentVariable::Parse(std::wstring entry) if (key.empty()) { - THROW_HR_WITH_USER_ERROR(E_INVALIDARG, "Environment variable key cannot be empty"); + THROW_HR_WITH_USER_ERROR(E_INVALIDARG, L"Environment variable key cannot be empty"); } if (std::any_of(key.begin(), key.end(), IsSpace)) @@ -126,9 +125,14 @@ std::vector EnvironmentVariable::ParseFile(std::wstring filePath) THROW_HR_WITH_USER_ERROR(E_INVALIDARG, std::format(L"Environment file '{}' does not exist", filePath)); } + std::ifstream file(filePath); + if (!file.is_open() || !file.good()) + { + THROW_HR_WITH_USER_ERROR(E_INVALIDARG, std::format(L"Environment file '{}' cannot be opened for reading", filePath)); + } + // Read the file line by line std::vector envVars; - std::ifstream file(filePath); std::string line; while (std::getline(file, line)) { diff --git a/test/windows/wslc/WSLCCLIEnvVarParserUnitTests.cpp b/test/windows/wslc/WSLCCLIEnvVarParserUnitTests.cpp index 1f0ef2f18..95194876f 100644 --- a/test/windows/wslc/WSLCCLIEnvVarParserUnitTests.cpp +++ b/test/windows/wslc/WSLCCLIEnvVarParserUnitTests.cpp @@ -96,7 +96,7 @@ class WSLCCLIEnvVarParserUnitTests const auto parsed = models::EnvironmentVariable::ParseFile(EnvTestFile.wstring()); - VERIFY_ARE_EQUAL(3, static_cast(parsed.size())); + VERIFY_ARE_EQUAL(3U, parsed.size()); VERIFY_ARE_EQUAL(L"KEY1=VALUE1", parsed[0]); VERIFY_ARE_EQUAL(L"KEY2=VALUE2", parsed[1]); VERIFY_ARE_EQUAL(L"WSLC_TEST_ENV_FROM_FILE=file_process_value", parsed[2]); diff --git a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp index 5c74ad408..d01c54f99 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp @@ -767,7 +767,7 @@ class WSLCE2EContainerCreateTests envFile << line << "\n"; } VERIFY_IS_TRUE(envFile.good()); - }; + } bool ContainsOutputLine(const std::vector& outputLines, const std::wstring& expectedLine) const { From fa688d4fac004936094242a31c65f486456d9649 Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Mon, 23 Mar 2026 11:18:14 -0700 Subject: [PATCH 16/17] Addressed comments --- src/windows/wslc/services/ContainerModel.cpp | 27 +++++--------------- src/windows/wslc/services/ContainerModel.h | 4 +-- 2 files changed, 9 insertions(+), 22 deletions(-) diff --git a/src/windows/wslc/services/ContainerModel.cpp b/src/windows/wslc/services/ContainerModel.cpp index 254674d0d..7099d6cc6 100644 --- a/src/windows/wslc/services/ContainerModel.cpp +++ b/src/windows/wslc/services/ContainerModel.cpp @@ -195,7 +195,7 @@ static inline bool IsSpace(wchar_t ch) return std::iswspace(ch) != 0; } -std::optional EnvironmentVariable::Parse(std::wstring entry) +std::optional EnvironmentVariable::Parse(const std::wstring& entry) { if (entry.empty() || std::all_of(entry.begin(), entry.end(), IsSpace)) { @@ -228,34 +228,21 @@ std::optional EnvironmentVariable::Parse(std::wstring entry) if (!value.has_value()) { - const DWORD valueLength = GetEnvironmentVariableW(key.c_str(), nullptr, 0); - if (valueLength > 0) - { - std::wstring envValue(valueLength, L'\0'); - GetEnvironmentVariableW(key.c_str(), envValue.data(), valueLength); - if (!envValue.empty() && envValue.back() == L'\0') - { - envValue.pop_back(); - } - - value = envValue; - } - else + wil::unique_hglobal_string envValue; + auto hr = wil::GetEnvironmentVariableW(key.c_str(), envValue); + if (FAILED(hr) || envValue.get() == nullptr) { return std::nullopt; } + + value = std::wstring(envValue.get()); } return std::format(L"{}={}", key, value.value()); } -std::vector EnvironmentVariable::ParseFile(std::wstring filePath) +std::vector EnvironmentVariable::ParseFile(const std::wstring& filePath) { - if (!std::filesystem::exists(filePath)) - { - THROW_HR_WITH_USER_ERROR(E_INVALIDARG, std::format(L"Environment file '{}' does not exist", filePath)); - } - std::ifstream file(filePath); if (!file.is_open() || !file.good()) { diff --git a/src/windows/wslc/services/ContainerModel.h b/src/windows/wslc/services/ContainerModel.h index 313594424..9833fabbf 100644 --- a/src/windows/wslc/services/ContainerModel.h +++ b/src/windows/wslc/services/ContainerModel.h @@ -72,8 +72,8 @@ struct ContainerInformation struct EnvironmentVariable { - static std::optional Parse(std::wstring entry); - static std::vector ParseFile(std::wstring filePath); + static std::optional Parse(const std::wstring& entry); + static std::vector ParseFile(const std::wstring& filePath); }; struct PublishPort From 1e5b7c271567dd3b2dc9109aa758a37d5bef4daf Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Mon, 23 Mar 2026 15:06:46 -0700 Subject: [PATCH 17/17] Fix test --- test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp index eaa20e8ab..32680c1d4 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp @@ -544,7 +544,8 @@ class WSLCE2EContainerCreateTests auto result = RunWslc(std::format( L"container run --rm --name {} --env-file ENV_FILE_NOT_FOUND {} env", WslcContainerName, DebianImage.NameAndTag())); - result.Verify({.Stderr = L"Environment file 'ENV_FILE_NOT_FOUND' does not exist\r\nError code: E_INVALIDARG\r\n", .ExitCode = 1}); + result.Verify( + {.Stderr = L"Environment file 'ENV_FILE_NOT_FOUND' cannot be opened for reading\r\nError code: E_INVALIDARG\r\n", .ExitCode = 1}); } TEST_METHOD(WSLCE2E_Container_Run_EnvFile_InvalidContent) @@ -686,7 +687,8 @@ class WSLCE2EContainerCreateTests result.Verify({.Stderr = L"", .ExitCode = S_OK}); result = RunWslc(std::format(L"container exec --env-file ENV_FILE_NOT_FOUND {} env", WslcContainerName)); - result.Verify({.Stderr = L"Environment file 'ENV_FILE_NOT_FOUND' does not exist\r\nError code: E_INVALIDARG\r\n", .ExitCode = 1}); + result.Verify( + {.Stderr = L"Environment file 'ENV_FILE_NOT_FOUND' cannot be opened for reading\r\nError code: E_INVALIDARG\r\n", .ExitCode = 1}); } TEST_METHOD(WSLCE2E_Container_RunInteractive_TTY)