Skip to content

fix: CallingAssembly.Find could adopt a test runner too (#600) - #605

Merged
jeremydmiller merged 1 commit into
mainfrom
gh600b/calling-assembly-runner
Aug 2, 2026
Merged

fix: CallingAssembly.Find could adopt a test runner too (#600)#605
jeremydmiller merged 1 commit into
mainfrom
gh600b/calling-assembly-runner

Conversation

@jeremydmiller

Copy link
Copy Markdown
Member

Follow-up to #602 / #604, on the other stack walk. Noted as out of scope on #602; picking it up now.

Stacked on #604 (which is stacked on #602). Review those first; this diff is the last commit only. Retarget to main once they merge.

The defect

AssemblyScanner.TheCallingAssembly() resolves through CallingAssembly.Find(), which ignored only System. and Microsoft. prefixes. Same exposure as #600: a scan configured from an async test can adopt the runner and then scan an assembly holding none of the suite's types.

But the implementation made it stranger than that. Find() rendered the stack as text (Environment.StackTrace) and guessed each frame's assembly by trying to Assembly.Load progressively shorter dotted prefixes of the method name:

var names = candidate.Split('.');
for (var i = names.Length - 2; i > 0; i--)
{
    var possibility = string.Join(".", names.Take(i).ToArray());
    try { assembly = Assembly.Load(new AssemblyName(possibility)); break; }
    catch { _misses.Add(possibility); }
}

That can only ever resolve an assembly whose name lines up with its namespace, which made it blind and wrong in different places:

  • it silently missed frames where they differ — xUnit's Xunit.v3 namespace vs its xunit.v3.core assembly;
  • it faithfully resolved, and adopted, ones that do line up — NUnit's NUnit.Framework namespace matches its nunit.framework assembly exactly.

So this is a real case, not a theoretical one, and which runner you use decides whether you hit it.

Three further problems in the same method:

  • _misses is a plain static List<string> mutated by every caller with no synchronisation — concurrent scans race on it.
  • stacktraceLine.Trim().Substring(3) throws ArgumentOutOfRangeException on any line shorter than three characters.
  • Every miss costs a throwing Assembly.Load.

The fix

Read the assembly off the frame, the way JasperFxOptions.DetermineCallingAssembly already does:

var frames = new StackTrace().GetFrames();

foreach (var frame in frames)
{
    var assembly = frame.GetMethod()?.DeclaringType?.Assembly;
    if (assembly is null || isSystemAssembly(assembly)) continue;
    return assembly;
}

return Assembly.GetEntryAssembly();

Exact, no speculative loads, no shared mutable state, and isSystemAssembly now shares JasperFxOptions.IsTestRunnerAssembly so the two walks agree on what a runner is.

Behaviour preserved

[IgnoreAssembly] is still honoured — that is precisely why Find() skips JasperFx's own frames, since JasperFx carries [assembly: IgnoreAssembly] (AssemblyScanner.cs:8). The three existing tests pin the "innermost non-ignored caller" semantics and are unchanged:

  • use_current_assembly → the test assembly
  • from_another_assemblyWidgets1
  • skip_ignore_assemblyWidgets5, skipping [IgnoreAssembly]-marked Widgets4

I did not add the #601 Critter Stack skip here. Find()'s contract is the immediate caller, [IgnoreAssembly] is the established opt-out for this API, and neither Wolverine nor Marten calls TheCallingAssembly() — so a name list would be redundant where an attribute already does the job better.

Tests

TestRunnerStandIn's namespace now matches its assembly name. That is required, not cosmetic: the text-based walk keys on namespace, so with a mismatched one the stand-in cannot reproduce the defect at all. The comment in the file explains it, and notes that the alignment mirrors NUnit rather than being a contrivance.

  • walks_past_a_test_runner_frame_out_to_the_calling_assembly / never_adopts_a_test_runner_as_the_calling_assembly — both fail against the old implementation (it returns xunit.v3.stackwalk.standin) and pass against the new one.
  • find_is_safe_to_call_concurrently — guards the _misses race.

Verified in Release as well as Debug, since these tests depend on frames the JIT is free to inline away and CI builds Release. All five suites green: CoreTests 542, EventTests 657, CommandLineTests 295, CodegenTests 419, EventStoreTests 72.

🤖 Generated with Claude Code

@jeremydmiller
jeremydmiller force-pushed the gh601/skip-critter-stack-assemblies branch from 4a31eb5 to 84be58d Compare August 2, 2026 13:29
@jeremydmiller
jeremydmiller force-pushed the gh600b/calling-assembly-runner branch from e302d11 to 65b5519 Compare August 2, 2026 13:31
@jeremydmiller
jeremydmiller changed the base branch from gh601/skip-critter-stack-assemblies to main August 2, 2026 13:36
Follow-up to the DetermineCallingAssembly fix, on the other stack walk.
AssemblyScanner.TheCallingAssembly() resolves through CallingAssembly.Find(), which only
ignored "System." and "Microsoft." prefixes -- so a scan configured from an async test
could adopt the runner and then scan an assembly holding none of the suite's types.

Find() rendered the stack as TEXT (Environment.StackTrace) and guessed each frame's
assembly by trying to Assembly.Load progressively shorter dotted prefixes of the method
name. That could only ever resolve an assembly whose name lined up with its namespace,
which made it both blind and wrong in different places: it silently missed frames whose
namespace differs from the assembly (xUnit's Xunit.v3 vs xunit.v3.core), and it faithfully
resolved -- and adopted -- ones that do line up. NUnit's NUnit.Framework namespace matches
its nunit.framework assembly exactly.

Read the assembly off the frame instead. Exact, no speculative loads, and it drops a
static List<string> of failed load attempts that every caller mutated without
synchronisation.

Behaviour is otherwise preserved: [IgnoreAssembly] is still honoured (that is why Find()
skips JasperFx's own frames -- JasperFx carries the attribute), and the three existing
tests pin the "innermost non-ignored caller" semantics unchanged.

The stand-in's namespace now matches its assembly name, because the text-based walk keys
on namespace and cannot be reproduced otherwise.

Verified in Release as well as Debug, since these tests depend on frames the JIT is free
to inline away.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant