Skip to content

Auto-promote scalar JSON values for arg-bearing CLI options (fixes #8830)#8836

Merged
Evangelink merged 2 commits into
microsoft:mainfrom
Evangelink:dev/amauryleve/fix-8830-scalar-promote
Jun 5, 2026
Merged

Auto-promote scalar JSON values for arg-bearing CLI options (fixes #8830)#8836
Evangelink merged 2 commits into
microsoft:mainfrom
Evangelink:dev/amauryleve/fix-8830-scalar-promote

Conversation

@Evangelink
Copy link
Copy Markdown
Member

Resolves #8830.

Problem

In testconfig.json, a bare scalar like

{ "platformOptions": { "commandLineOptions": { "results-directory": "TestResults" } } }

was read by JsonConfigurationProvider as a presence marker via bool.TryParse for any option whose schema requires an argument. For an arg-bearing option, "timeout": "true" therefore enabled the option with zero args instead of passing "true" as the first argument value — a quiet foot-gun.

Fix

Normalize _singleValueData once at startup using the registered option registry: when an option has Arity.Min >= 1, rewrite the bare scalar key

commandLineOptions:<name>

into the indexed form

commandLineOptions:<name>:0

so the value is consistently surfaced as the first argument by both EnumerateCommandLineOptions and TryGetCommandLineOptionFromProviders.

Optional-arg options (Min=0, Max>=1) remain ambiguous by design and are left untouched (workaround: use the array form ["true"]). Zero-arity and unknown options are also left as-is — the existing validator catches unknown options.

Wire-up

TestHostBuilder.CommonServices.cs builds an OrdinalIgnoreCase option lookup from the system + extension command-line providers and calls the new AggregatedConfiguration.NormalizeJsonCommandLineOptionScalars before EnumerateJsonCommandLineOptions, so the validator and any other consumer sees the rewritten form.

Tests

Added 9 new [TestMethod]s in JsonCommandLineOptionsTests.cs covering:

  • arg-bearing scalar promoted
  • "true" / "false" strings on arg-bearing options promoted (regression for the original ambiguity)
  • non-bool scalar promoted
  • zero-arity option left as presence marker
  • optional-arg option left untouched (still ambiguous, documented workaround)
  • unknown option untouched
  • already-indexed JSON untouched
  • case-insensitive option lookup
  • no-op when no JSON source is registered

All 380 existing Configuration / CommandLine tests pass; full net9.0 build clean with -warnaserror.

…crosoft#8830)

testconfig.json bare scalars like "results-directory": "TestResults" were
read by JsonConfigurationProvider as presence markers (via bool.TryParse) for
options whose schema requires an argument. For an arg-bearing option
`"timeout": "true"` ended up enabling the option with zero args instead of
passing `"true"` as the first argument value.

Normalize `_singleValueData` at startup using the registered option
registry: when an option has `Arity.Min >= 1`, rewrite the bare scalar key
`commandLineOptions:<name>` into the indexed form `commandLineOptions:<name>:0`
so the value is consistently surfaced as the first argument by both
`EnumerateCommandLineOptions` and `TryGetCommandLineOptionFromProviders`.

Optional-arg options (`Min=0, Max>=1`) remain ambiguous by design and are
left untouched (workaround: use the array form `["true"]`). Zero-arity and
unknown options are also left as-is.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 4, 2026 23:13
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a long-standing ambiguity in testconfig.json parsing where scalar JSON values for arg-bearing CLI options (e.g., "timeout": "true") could be misinterpreted as a presence marker instead of being treated as the first argument. It does so by normalizing JSON scalar entries into the canonical indexed configuration shape (commandLineOptions:<name>:0) once the option registry is available.

Changes:

  • Added JSON-scalar normalization logic in JsonConfigurationProvider and surfaced it via AggregatedConfiguration.NormalizeJsonCommandLineOptionScalars.
  • Wired normalization into TestHostBuilder so it runs before JSON options are enumerated/validated.
  • Added unit tests covering scalar promotion behavior, case-insensitive lookup, and no-op behavior when no JSON provider is present.
Show a summary per file
File Description
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Configuration/JsonCommandLineOptionsTests.cs Adds coverage for scalar-to-indexed normalization across arity and casing scenarios.
src/Platform/Microsoft.Testing.Platform/Hosts/TestHostBuilder.CommonServices.cs Builds an option-name lookup and invokes JSON normalization before enumerating JSON CLI options.
src/Platform/Microsoft.Testing.Platform/Configurations/JsonConfigurationProvider.cs Implements the scalar key rewrite (commandLineOptions:<name>...:<name>:0) for Arity.Min >= 1.
src/Platform/Microsoft.Testing.Platform/Configurations/AggregatedConfiguration.cs Exposes a single entry point to normalize JSON command-line scalar entries when a JSON provider is present.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 1

Comment thread src/Platform/Microsoft.Testing.Platform/Hosts/TestHostBuilder.CommonServices.cs Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 5, 2026 07:28
@Evangelink Evangelink enabled auto-merge (squash) June 5, 2026 07:28
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new

@Evangelink Evangelink merged commit db59175 into microsoft:main Jun 5, 2026
31 checks passed
@Evangelink Evangelink deleted the dev/amauryleve/fix-8830-scalar-promote branch June 5, 2026 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[MTP] testconfig.json: auto-promote scalar values for arg-bearing options

3 participants