Re-enable binder tracing tests disabled during reflection invoke work

[Copilot]

The tests pass on main and the original issue no longer seems to be a problem.

cc @dotnet/appmodel @AaronRobinsonMSFT

• FixesUpdate BinderTracingTest.AssemblyLoadContextResolving_ReturnNull to not assume reflection semantics #68521

[Copilot]

Copilot wasn't able to review any files in this pull request.

[azure-pipelines]

Azure Pipelines:
16 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

Review details

Suppressed comments (2)

src/tests/Loader/binding/tracing/BinderTracingTest.cs:209

• ValidateSingleBind now validates each candidate by calling ValidateBindOperation inside a predicate. This uses exceptions for control flow and also triggers the Console.WriteLine in ValidateBindOperation for every candidate, which can inflate test output and slow the run when multiple events are present. Consider a fast path for the common single-event case, and count matches without allocating a list.

 Console.WriteLine($"[{DateTime.Now:T}] Validating bind operation for {assemblyName}...");
BindOperation[] binds = listener.WaitAndGetEventsForAssembly(assemblyName);
List<BindOperation> matchingBinds = binds
.Where(bind => Helpers.ValidateBindOperationOrReturnFalse(expected, bind))
.ToList();

src/tests/Loader/binding/tracing/Helpers.cs:55

• The helper name ValidateBindOperationOrReturnFalse isn’t idiomatic for a boolean-returning “try” pattern, and it reads like control-flow rather than intent. Consider renaming to a Try* name (e.g., TryValidateBindOperation) to make predicate-style usage clearer.

 internal static bool ValidateBindOperationOrReturnFalse(BindOperation expected, BindOperation actual)
{
try
{
ValidateBindOperation(expected, actual);
return true;
}
catch (Xunit.Sdk.XunitException)
{
return false;
}
}

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

Review details

Suppressed comments (1)

src/tests/Loader/binding/tracing/BinderTracingTest.EventHandlers.cs:42

• PR description says "Fixes #68521", but the suite still contains at least one BinderTest explicitly skipped for that same issue (e.g., AssemblyLoadFromResolveHandler_MissingDependency is still annotated with activeIssue: #68521). Either remove the "Fixes" claim from the PR description or update/unskip the remaining test(s) and adjust expectations so the issue is actually resolved end-to-end.

 List<HandlerInvocation> invocations = handlers.Invocations.Where(invocation => Helpers.AssemblyNamesMatch(invocation.AssemblyName, assemblyName)).ToList();
Assert.Single(invocations);
Assert.Empty(handlers.Binds);

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

Review details

Suppressed comments (2)

src/tests/Loader/binding/tracing/BinderTracingTest.ResolutionFlow.cs:76

• PR description says BinderTracing resolving callback expectations were updated (e.g., filtering recorded callbacks to the requested AssemblyName), but this change only removes the ActiveIssue gate from several [BinderTest]s. I couldn't find any corresponding expectation updates in the BinderTracing tests, so please either include the expected-behavior adjustments in this PR or keep these tests behind ActiveIssue until the fix is present.

 [BinderTest(isolate: true, testSetup: nameof(LoadSubdirectoryAssembly_InstanceALC))]

src/tests/Loader/binding/tracing/BinderTracingTest.EventHandlers.cs:278

• The inline comment says Emit-based Invoke causes an extra load, but the test is no longer gated by ActiveIssue. Rewording this to describe why additionalLoadsToTrack is present avoids implying the test is expected to fail.

 [BinderTest(isolate: true, additionalLoadsToTrack: new string[] { "AssemblyToLoadDependency" })] // Emit-based Invoke causes an extra load.

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[elinor-fung]

/ba-g build analysis misclassifying known test failures - dotnet/arcade#17340