Skip to content

Clarify no-op nature of duplicate test definition name update in TRX report engine#7599

Closed
Copilot wants to merge 1 commit intodev/ygerges/trx-displayfrom
copilot/sub-pr-7595-again
Closed

Clarify no-op nature of duplicate test definition name update in TRX report engine#7599
Copilot wants to merge 1 commit intodev/ygerges/trx-displayfrom
copilot/sub-pr-7595-again

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 24, 2026

The auto-reviewer flagged a potential inconsistency between uniqueTestDefinitionTestIds and the testDefinitions XElement when a second occurrence of a test ID provides an explicit TrxTestDefinitionName. The concern was that the dictionary update at lines 586-590 could diverge from the already-written XElement.

Analysis

The concern is not valid. When isExplicitlyProvided is true, the guard at line 580 already enforces name equality before reaching line 586:

// Lines 580-583: throws if either name is explicit AND names differ
if ((isExplicitlyProvided || existing.ExistingIsExplicitlyProvided) &&
    existing.ExistingTestDefinitionName != testDefinitionName)
{
    throw new InvalidOperationException(...);
}

// Lines 586-590: only reachable when names are guaranteed equal
if (!existing.ExistingIsExplicitlyProvided && isExplicitlyProvided)
{
    // testDefinitionName == existing.ExistingTestDefinitionName here — only flag changes
    uniqueTestDefinitionTestIds[id] = (testDefinitionName, true);
}

Since differing names trigger an exception at line 583, line 589 is only ever reached when the names are identical. The dictionary update is name-identical; the only mutation is flipping ExistingIsExplicitlyProvided from false to true. No XElement/dictionary name inconsistency is possible.


📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.

Copilot AI changed the title [WIP] [WIP] Address feedback on test definition names and exception handling in TRX fix Clarify no-op nature of duplicate test definition name update in TRX report engine Mar 24, 2026
Copilot AI requested a review from Youssef1313 March 24, 2026 11:35
@Youssef1313 Youssef1313 deleted the copilot/sub-pr-7595-again branch March 25, 2026 10:06
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.

2 participants