Skip to content

Test isolation violation: Deedle static initialisation is unsafe under MSTest class-level parallelism in QuickFiler.Test #877

Description

@drmoisan

Ruling that frames this issue

Tests must always run in parallel. A suite that requires serial execution has already violated unit-test isolation. scripts/vscode/TaskMaster.cli.runsettings with Workers=0, Scope=ClassLevel is CORRECT and stays as it is. The failing tests are the defect.

PR #878, which proposed reducing Workers to 1, was closed unmerged as a mask. [DoNotParallelize], per-class opt-outs, retries and timing tolerances are out for the same reason.

Acceptance test

QuickFiler.Test passes with Workers=0, Scope=ClassLevel unchanged and /Settings: still passed.

Symptom

Three tests in QuickFiler.Controllers.Tests.QfcInitEmailQueueZeroBatchTests fail only under class-level parallelism, with TypeInitializationException for Deedle.Reflection, then for <StartupCode$Deedle>.$FrameUtils, then FileNotFoundException: netstandard, Version=2.1.0.0. Elapsed 9 ms, under 1 ms, 1 ms — assembly-load failure at static-initialiser time, not an assertion failure.

A FIRST DIAGNOSIS THAT WAS REFUTED — recorded so it is not re-proposed

The first theory was that QuickFiler.Test depends on a process-wide AppDomain.CurrentDomain.AssemblyResolve fallback installed by UtilitiesCS.Test's [AssemblyInitialize], and that under parallelism a QuickFiler.Test class can reach Deedle before that sibling initialiser runs.

It is refuted by a measurement that already existed. Item 872's Phase 0 artifact records:

Command: vstest.console.exe QuickFiler.Test\bin\Debug\QuickFiler.Test.dll /InIsolation ...
EXIT_CODE: 0
TotalTests: 1394
Passed: 1394
Failed: 0

with no /Settings: passed. That run loaded QuickFiler.Test alone, so UtilitiesCS.Test's [AssemblyInitialize] never ran and its resolver was absent — yet Deedle resolved netstandard 2.1 and every test passed.

If the suite passes serially with no resolver installed, the absence of the resolver cannot be what makes it fail. The cross-assembly ordering dependency is not the mechanism.

These facts remain true and verified, they simply do not explain the failure: UtilitiesCS.Test does install such a resolver, QuickFiler.Test does not, and neither app.config carries a netstandard redirect (73 and 76 bindingRedirect entries, zero netstandard).

Surviving hypothesis, NOT yet tested

Both arms of the isolating measurement ran QuickFiler.Test alone. The only variable between them is the runsettings, i.e. MSTest class-level parallelism.

So the defect is a genuine concurrency fault in the first-touch static initialisation of Deedle. The shared mutable static state is Deedle's own type initialiserDeedle.Reflection and <StartupCode$Deedle>.$FrameUtils — which is process-global and not safe against concurrent first touch. Under Scope=ClassLevel several test classes race to trigger it; one fails, and the CLR caches the failed initialiser for the process lifetime, which is why repeated runs then look deterministic and why a no-coverage control was never a control.

Candidate fix direction, to be validated not assumed

Force Deedle's type initialisation once, single-threaded, in QuickFiler.Test's own [AssemblyInitialize], before any parallel class runs. That reduces no parallelism, uses no [DoNotParallelize] and changes no worker count: it makes the shared static state deterministic rather than racing, which is the test assembly owning its side of the contract.

Whether that is sufficient is unverified. It must be measured.

Required before implementing

Phase 0 must measure the mechanism rather than inherit it:

  1. QuickFiler.Test alone, WITHOUT the runsettings — expected pass, reproduces the 1394/1394 baseline.
  2. QuickFiler.Test alone, WITH the runsettings — expected 3 failures. This is the reproduction.
  3. Whether more than one test class touches Deedle, and which types are on the first-touch path.

If (2) does not reproduce, stop and report; the mechanism is not understood and no fix should be written.

Prohibited

No change to TaskMaster.cli.runsettings; no Workers change; no weakening of the Parallelize block; no [DoNotParallelize]; no dropping /Settings:; no retries; no timing tolerance; no sleeps; no test reordering; and no [ClassInitialize] hack that merely front-runs the race in one class.

Classification

Test hygiene rather than a production thread-safety bug, on current evidence: production loads Deedle through a single-threaded startup path and resolves bindings via TaskMaster.exe.config. This should be re-examined if the surviving hypothesis is confirmed, because a static initialiser that is unsafe under concurrent first touch could also be reached concurrently in production on a many-core machine.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions