Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using Microsoft.Testing.Platform.Configurations;
using Microsoft.Testing.Platform.Extensions.CommandLine;
using Microsoft.Testing.Platform.Extensions.OutputDevice;
using Microsoft.Testing.Platform.Helpers;
Expand All @@ -17,17 +18,31 @@ internal sealed class CommandLineHandler : ICommandLineHandler, ICommandLineOpti

private readonly ITestApplicationModuleInfo _testApplicationModuleInfo;
private readonly IRuntimeFeature _runtimeFeature;
private readonly IConfiguration? _configuration;

public CommandLineHandler(CommandLineParseResult parseResult, IReadOnlyCollection<ICommandLineOptionsProvider> extensionsCommandLineOptionsProviders,
IReadOnlyCollection<ICommandLineOptionsProvider> systemCommandLineOptionsProviders, ITestApplicationModuleInfo testApplicationModuleInfo,
IRuntimeFeature runtimeFeature)
: this(parseResult, extensionsCommandLineOptionsProviders, systemCommandLineOptionsProviders, testApplicationModuleInfo, runtimeFeature, configuration: null)
{
}

// Option C (issue #6349): the unified read model is keyed on IConfiguration. When the
// platform owns the construction (TestHostBuilder -> CommandLineManager.BuildAsync), the
// configuration is supplied so IsOptionSet/TryGetOptionArgumentList consult both the CLI
// and testconfig.json. When configuration is null (e.g. ad-hoc unit-test construction),
// we fall back to the legacy parse-result-only behavior.
internal CommandLineHandler(CommandLineParseResult parseResult, IReadOnlyCollection<ICommandLineOptionsProvider> extensionsCommandLineOptionsProviders,
IReadOnlyCollection<ICommandLineOptionsProvider> systemCommandLineOptionsProviders, ITestApplicationModuleInfo testApplicationModuleInfo,
IRuntimeFeature runtimeFeature, IConfiguration? configuration)
{
ParseResult = parseResult;
ExtensionsCommandLineOptionsProviders = extensionsCommandLineOptionsProviders;
SystemCommandLineOptionsProviders = systemCommandLineOptionsProviders;
CommandLineOptionsProviders = systemCommandLineOptionsProviders.Union(extensionsCommandLineOptionsProviders);
_testApplicationModuleInfo = testApplicationModuleInfo;
_runtimeFeature = runtimeFeature;
_configuration = configuration;
}

public IEnumerable<ICommandLineOptionsProvider> CommandLineOptionsProviders { get; }
Expand Down Expand Up @@ -214,10 +229,14 @@ async Task DisplayRegisteredToolsInfoAsync(IOutputDevice outputDevice, IReadOnly
}

public bool IsOptionSet(string optionName)
=> ParseResult.IsOptionSet(optionName);

public bool TryGetOptionArgumentList(string optionName, [NotNullWhen(true)] out string[]? arguments) =>
ParseResult.TryGetOptionArgumentList(optionName, out arguments);
=> _configuration is not null
? _configuration.IsCommandLineOptionSet(optionName)
: ParseResult.IsOptionSet(optionName);

public bool TryGetOptionArgumentList(string optionName, [NotNullWhen(true)] out string[]? arguments)
=> _configuration is not null
? _configuration.TryGetCommandLineOptionArguments(optionName, out arguments)
: ParseResult.TryGetOptionArgumentList(optionName, out arguments);
Comment on lines +232 to +239

public Task<bool> IsEnabledAsync() => Task.FromResult(false);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using Microsoft.Testing.Platform.Configurations;
using Microsoft.Testing.Platform.Extensions;
using Microsoft.Testing.Platform.Extensions.CommandLine;
using Microsoft.Testing.Platform.Helpers;
Expand Down Expand Up @@ -36,7 +37,7 @@ public void AddProvider(Func<IServiceProvider, ICommandLineOptionsProvider> comm
_commandLineProviderFactory.Add(commandLineProviderFactory);
}

internal async Task<CommandLineHandler> BuildAsync(CommandLineParseResult parseResult, IServiceProvider serviceProvider)
internal async Task<CommandLineHandler> BuildAsync(CommandLineParseResult parseResult, IServiceProvider serviceProvider, IConfiguration configuration)
{
List<ICommandLineOptionsProvider> commandLineOptionsProviders = [];
foreach (Func<IServiceProvider, ICommandLineOptionsProvider> commandLineProviderFactory in _commandLineProviderFactory)
Expand All @@ -62,6 +63,6 @@ commandLineOptionsProvider is IToolCommandLineOptionsProvider toolCommandLineOpt
];

return new CommandLineHandler(parseResult, commandLineOptionsProviders,
systemCommandLineOptionsProviders, _testApplicationModuleInfo, _runtimeFeature);
systemCommandLineOptionsProviders, _testApplicationModuleInfo, _runtimeFeature, configuration);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,10 @@ private static ValidationResult ValidateNoUnknownOptions(
}

StringBuilder? stringBuilder = null;
// OPTION C LIMITATION (issue #6349): unknown-option detection walks parseResult.Options
// only. Unknown keys under the "commandLineOptions" section of testconfig.json are
// silently ignored. Should be augmented by enumerating that JSON section (see
// AggregatedConfiguration.GetTestConfigJsonSection) once a strict schema is decided.
foreach (CommandLineParseOption optionRecord in parseResult.Options)
{
if (!validOptionNames.Contains(optionRecord.Name))
Expand All @@ -223,6 +227,18 @@ private static ValidationResult ValidateOptionsArgumentArity(
CommandLineParseResult parseResult,
Dictionary<string, (ICommandLineOptionsProvider Provider, CommandLineOption Option)> providerAndOptionByOptionName)
{
// OPTION C LIMITATION (issue #6349): arity is validated only against options that
// appear on the CLI (parseResult.Options). Options sourced from testconfig.json's
// "commandLineOptions" section are NOT seen by this validator. Reach parity by either
// (a) synthesizing CommandLineParseOption entries from the JSON layer before this call
// (closer to Option B), or (b) iterating the merged IConfiguration view here.
//
// Concrete consequences of skipping JSON validation (seen at runtime as exceptions
// rather than friendly CLI errors):
// - TestHostBuilder.CommonServices.cs (~L217) does args[0] on TIMEOUT — JSON
// "timeout": true (arity=0) yields an empty array and throws IndexOutOfRange.
// - PlatformCommandLineProvider.cs (~L237) does int.Parse on the exit-on-process-exit
// argument — JSON "exit-on-process-exit": "abc" throws FormatException.
StringBuilder? stringBuilder = null;
foreach (IGrouping<string, CommandLineParseOption> groupedOptions in parseResult.Options.GroupBy(x => x.Name))
{
Expand Down Expand Up @@ -264,6 +280,9 @@ private static async Task<ValidationResult> ValidateOptionsArgumentsAsync(
{
ApplicationStateGuard.Ensure(parseResult is not null);

// OPTION C LIMITATION (issue #6349): per-option ValidateOptionArgumentsAsync runs only
// for CLI-supplied values. A value coming from testconfig.json bypasses the provider's
// own argument validator. Same fix shape as the arity limitation above.
StringBuilder? stringBuilder = null;
foreach (CommandLineParseOption optionRecord in parseResult.Options)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,86 @@ public string? this[string key]
public /* for testing */ void SetCurrentWorkingDirectory(string workingDirectory) =>
_currentWorkingDirectory = workingDirectory;

/// <summary>
/// Resolves the value of a command-line option across all registered providers at the
/// <em>option</em> granularity (not per-key), so that CLI provider entries fully shadow
/// JSON entries for the same option even when the storage shapes don't overlap (e.g.,
/// CLI <c>--list-tests</c> with no args vs. JSON <c>"list-tests": ["json"]</c>).
/// </summary>
/// <remarks>
/// <para>
/// Providers are walked in registration order; the first one that has any data for
/// <paramref name="optionName"/> wins and its values are returned exclusively. A bare
/// boolean <c>"false"</c> at a winning provider is treated as an explicit disable and
/// short-circuits the search.
/// </para>
/// <para>
/// Provider contract assumptions for the <c>commandLineOptions:*</c> section (Option C,
/// issue #6349):
/// <list type="bullet">
/// <item><description><c>TryGet</c> returning <c>true</c> with a <c>null</c> value is
/// treated as if the key were absent at this provider — a custom provider that wants
/// to explicitly shadow lower providers must return either the boolean string
/// <c>"False"</c> (disable) or a non-null argument value.</description></item>
/// <item><description>Indexed argument entries are required to be contiguous starting
/// at <c>:0</c>. A gap stops collection — sparse arrays will silently truncate. The
/// JSON provider always emits contiguous indices for arrays.</description></item>
/// </list>
/// </para>
/// </remarks>
/// <param name="optionName">Option name with or without leading <c>--</c>.</param>
/// <param name="isSet">Set to <c>true</c> if any provider reports the option as set.</param>
/// <param name="arguments">Empty for zero-arity / disabled, otherwise the argument list
/// from the winning provider.</param>
/// <returns><c>true</c> if any provider had any data for the option (even an explicit
/// disable); <c>false</c> if no provider mentioned the option.</returns>
internal bool TryGetCommandLineOptionFromProviders(string optionName, out bool isSet, out string[] arguments)
{
string baseKey = PlatformConfigurationConstants.CommandLineOptionsSectionName
+ PlatformConfigurationConstants.KeyDelimiter
+ optionName.Trim(CommandLineParseResult.OptionPrefix);

foreach (IConfigurationProvider provider in _configurationProviders)
{
// Try indexed entries first (multi/single value options).
List<string>? collected = null;
int index = 0;
while (provider.TryGet(baseKey + PlatformConfigurationConstants.KeyDelimiter + index.ToString(CultureInfo.InvariantCulture), out string? indexed)
&& indexed is not null)
{
collected ??= [];
collected.Add(indexed);
index++;
}

if (collected is { Count: > 0 })
{
isSet = true;
arguments = [.. collected];
return true;
}

// Bare key (zero-arity presence marker, or a scalar JSON value).
if (provider.TryGet(baseKey, out string? bare) && bare is not null)
{
if (bool.TryParse(bare, out bool boolValue))
{
isSet = boolValue;
arguments = [];
return true;
}

isSet = true;
arguments = [bare];
return true;
}
}

isSet = false;
arguments = [];
return false;
}

/// <summary>
/// Returns the immediate (one-level) string entries declared under <paramref name="sectionName"/>
/// in the loaded testconfig.json file. Returns an empty list if no JSON configuration source is
Expand Down Expand Up @@ -102,6 +182,35 @@ private string GetResultsDirectoryCore(CommandLineParseResult commandLineParseRe
// When tests are retried, we re-run the executable again, but we pass --results-directory containing the folder where the current retry attempt
// results should be stored. But we will also still pass configuration file (e.g, runsettings via --settings) that the user originally specified.
// In that case, we will want to respect --results-directory.
//
// Option C (issue #6349): when the CLI configuration source is registered (the default
// host path), consult the unified command-line view first so a value supplied either via
// CLI or via testconfig.json's "commandLineOptions:results-directory" is honored with the
// same priority. CLI still wins over JSON because the CLI provider is Order=0.
//
// When the CLI source is NOT registered (test-only callers that build AggregatedConfiguration
// by hand), fall back to the legacy parseResult-first behavior so we don't silently demote
// CLI precedence behind any other provider that happens to expose the same key.
bool cliSourceRegistered = false;
foreach (IConfigurationProvider provider in _configurationProviders)
{
if (provider is CommandLineConfigurationProvider)
{
cliSourceRegistered = true;
break;
}
}

if (cliSourceRegistered
&& TryGetCommandLineOptionFromProviders(PlatformCommandLineProvider.ResultDirectoryOptionKey, out bool resultDirIsSet, out string[] resultDirArgs)
&& resultDirIsSet
&& resultDirArgs.Length > 0)
{
return resultDirArgs[0];
}

// Fallback for legacy callers without the CLI source registered: read directly from the
// original parse result so explicit CLI wins over any custom provider for this option.
if (commandLineParseResult.TryGetOptionArgumentList(PlatformCommandLineProvider.ResultDirectoryOptionKey, out string[]? resultDirectoryArg))
{
return resultDirectoryArg[0];
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using Microsoft.Testing.Platform.CommandLine;

namespace Microsoft.Testing.Platform.Configurations;

/// <summary>
/// <see cref="IConfigurationProvider"/> that exposes the parsed CLI options under the
/// <c>commandLineOptions:*</c> section.
/// </summary>
/// <remarks>
/// <para>Storage layout (mirrors the layout that <c>JsonConfigurationFileParser</c> produces for
/// the <c>commandLineOptions</c> JSON section, so the same lookup helpers serve both sources):</para>
/// <list type="bullet">
/// <item><description><b>Zero-arity option present</b> (e.g. <c>--hangdump</c>): stores
/// <c>commandLineOptions:&lt;name&gt;</c> = <c>"true"</c> as a presence marker.</description></item>
/// <item><description><b>Single- or multi-value option</b> (e.g. <c>--hangdump-timeout 5m</c> or
/// <c>--filter-uid a b</c>): stores one entry per argument under
/// <c>commandLineOptions:&lt;name&gt;:&lt;index&gt;</c>.</description></item>
/// </list>
/// <para>
/// Option names are stored without the leading <c>--</c> prefix and compared case-insensitively
/// to match <see cref="CommandLineParseResult.IsOptionSet(string)"/> semantics.
/// </para>
/// <para>
/// An option that appears multiple times on the command line is flattened into a single sequence
/// of arguments (matching <see cref="CommandLineParseResult.TryGetOptionArgumentList"/>), so the
/// resulting key/value layout is identical to what a JSON array under the same option name would
/// produce.
/// </para>
/// </remarks>
internal sealed class CommandLineConfigurationProvider : IConfigurationProvider
{
private readonly Dictionary<string, string?> _data = [with(StringComparer.OrdinalIgnoreCase)];

public CommandLineConfigurationProvider(CommandLineParseResult commandLineParseResult)
{
string sectionPrefix = PlatformConfigurationConstants.CommandLineOptionsSectionName + PlatformConfigurationConstants.KeyDelimiter;

// Group by option name so that --filter-uid a --filter-uid b is exposed as a single
// contiguous indexed list (commandLineOptions:filter-uid:0=a, :1=b), matching the existing
// TryGetOptionArgumentList flattening behavior.
foreach (IGrouping<string, CommandLineParseOption> grouping in commandLineParseResult.Options.GroupBy(o => o.Name, StringComparer.OrdinalIgnoreCase))
{
string baseKey = sectionPrefix + grouping.Key;
int index = 0;
foreach (CommandLineParseOption option in grouping)
{
foreach (string argument in option.Arguments)
{
_data[baseKey + PlatformConfigurationConstants.KeyDelimiter + index.ToString(CultureInfo.InvariantCulture)] = argument;
index++;
}
}

// Zero-arity flag: surface a boolean presence marker under the bare key so that
// IsCommandLineOptionSet returns true. Multi-value options intentionally do NOT
// populate the bare key (the indexed entries above represent the value).
if (index == 0)
{
_data[baseKey] = bool.TrueString;
}
}
}

public Task LoadAsync() => Task.CompletedTask;

public bool TryGet(string key, out string? value) => _data.TryGetValue(key, out value);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using Microsoft.Testing.Platform.CommandLine;

namespace Microsoft.Testing.Platform.Configurations;

/// <summary>
/// <see cref="IConfigurationSource"/> that exposes the parsed CLI options through the unified
/// <see cref="IConfiguration"/> read model (Option C of issue #6349).
/// </summary>
/// <remarks>
/// <para>
/// Registered with <see cref="Order"/> 0 so that CLI-provided values always win against values
/// from environment variables (<see cref="EnvironmentVariablesConfigurationSource"/>, Order 1) and
/// from <c>testconfig.json</c> (<see cref="JsonConfigurationSource"/>, Order 3). See
/// <see cref="AggregatedConfiguration"/> for the first-hit-wins lookup semantics.
/// </para>
/// <para>
/// The provider built by this source mirrors the JSON-flattened layout, allowing both the CLI and
/// <c>commandLineOptions</c> in <c>testconfig.json</c> to be merged transparently. See
/// <see cref="CommandLineConfigurationProvider"/> for the key shape.
/// </para>
/// </remarks>
internal sealed class CommandLineConfigurationSource : IConfigurationSource
{
public string Uid => nameof(CommandLineConfigurationSource);

public string Version => PlatformVersion.Version;

// Can be empty string because it's not used in the UI.
public string DisplayName => string.Empty;

// Can be empty string because it's not used in the UI.
public string Description => string.Empty;

// Lowest Order so the CLI wins against env vars (1) and testconfig.json (3).
public int Order => 0;

public Task<bool> IsEnabledAsync() => Task.FromResult(true);

public Task<IConfigurationProvider> BuildAsync(CommandLineParseResult commandLineParseResult)
=> Task.FromResult<IConfigurationProvider>(new CommandLineConfigurationProvider(commandLineParseResult));
}
Loading
Loading