[Review agent] Require exact test-endpoint reachability and non-copied fixture delta before keeping test-asset findings

Open
  • Is something wrong?
#70,336 1 comment 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
42/100
Issue type
Feature
Clarity
Mostly clear
Activity status
Active
Tech stack
csharp
Domain
testing

Research direction

Start with the review-candidate-verifier behavior described in the issue and trace how it evaluates test-asset findings, especially trigger, effect, and oldVsNew. Compare the two named Remote MSAL and Remote Authentication startup fixtures and inspect the active E2E test path. Done means findings require evidence that the exact code is reached with an observable test effect, and copied fixture code is evaluated against its existing baseline.

Written by the indexing model from the issue text.

Description

area-infrastructure
Summary

During Round 2 review of #69721, a reviewer reported and review-candidate-verifier returned KEEP on a string.IsNullOrEmpty(lastCode) && code != lastCode check at src/Components/test/testassets/Components.TestServer/RemoteMsalAuthenticationStartup.cs:77 (/subdir/oidc/token).

Root Cause
  1. Unreached endpoint in the executed test: While RemoteMsalAuthenticationTest hosts TrimmingServerFixture<RemoteMsalAuthenticationStartup>, the active MSAL E2E test only verifies that AuthenticationService.signIn is invoked (__msalSignInCalled) and stops before completing the authorization-code exchange against /subdir/oidc/token. The /subdir/oidc/token branch therefore has no effect on any executed test outcome.
  2. Verbatim copy of pre-existing fixture code (oldVsNew): RemoteMsalAuthenticationStartup.cs:77 is a verbatim copy of existing fixture code in src/Components/test/testassets/Components.TestServer/RemoteAuthenticationStartup.cs:82 on main, which review-candidate-verifier treated as oldVsNew: "n/a" because it appeared in a newly added startup file.
Proposed Fix
  • For findings in testassets/** and test/**, require trigger and effect to prove that an executed test actually reaches the exact route, callback, or branch and that the flaw can affect an observable test outcome (false pass, false failure, flakiness, or masking a product bug under test).
  • When a new test fixture copies helper/endpoint code verbatim from an existing fixture in the repository, require oldVsNew to account for the existing fixture baseline rather than treating the copied helper as a brand-new defect.

Tracked under #70231.

Dominant language
C#
Stars
38.5k
Forks
11.6k
Avg merge
3d 1h
Merged PRs (30d)
289

Getting set up

Open in Codespaces

Starts the project's dev container in your browser, under your own GitHub account.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from dotnet/aspnetcore

All issues in dotnet/aspnetcore

Similar issues

More C# issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.