Skip to content

Bug: tests-depend-on-uncontrolled-environment #931

Closed

Description

@drmoisan
  • Work Mode: full-bug

Summary

Consolidates #905 and #906. The shared root cause: a unit test depends on environment state it does not control, so its result depends on scheduling or on other processes rather than on the code under test.

  1. Bug: Bug: Sibling breadcrumb dispatcher tests share the Task.Run distinct-thread assumption fixed in #900 #905: tests use Task.Run as the "other thread". Task.Run guarantees only a thread-pool thread, never a different one, so under parallel execution the guard under test can go unexercised. PR fix(quickfiler): run breadcrumb thread-affinity worker tests on a dedicated thread #904 (Bug: Bug: Breadcrumb thread-affinity tests assume Task.Run yields a distinct thread #900) fixed two instances. Remaining:
    • QuickFiler.Test/Viewers/ItemViewerBreadcrumbThreadAffinityTests.cs:332. The file is 490 lines, so the fix requires a split to stay under 500.
    • Candidates to triage:
      • BreadcrumbSelectorToggleUiBoundaryTests.cs:75
      • BreadcrumbPopupControlDispatchTests.cs:29,111
      • BreadcrumbPopupBoundaryCoverageTests.cs:58
      • BreadcrumbPopupBoundaryCoverageTests.Part2.cs:192
      • BreadcrumbUiThreadDispatchTests.cs:90,301
    • Task.Run(() => tcs.SetResult(...)) calls that only complete a task are not affected.
  2. Bug: Bug: FileInfoWrapper_Tests.OpenRead opens the repository's own TaskMaster.sln #906: UtilitiesCS.Test/HelperClasses/FileInfoWrapper_Tests.cs:56-62 opens the repository's own TaskMaster.sln, found by GetSolutionFile() at lines 340-352, as a fixture. Resident MSBuild node-reuse workers can hold that file open, so the outcome depends on build history.

Environment

  • OS/version: Windows 11 Pro 10.0.26200
  • Python version: not applicable (C#, MSTest, net48)
  • Command/flags used: parallel regime /Settings:TaskMaster.runsettings (Workers 0, Scope ClassLevel)
  • Data source or fixture: files listed above, main at 177b6d78e

Steps to Reproduce

  1. Inspect the cited Task.Run sites and confirm that each asserts a thread-identity property against the thread it obtained.
  2. Inspect FileInfoWrapper_Tests.GetSolutionFile() and confirm it resolves the repository's own solution file.

Expected Behavior

  • A test that needs a distinct thread uses a dedicated Thread that is joined, and asserts inside that thread that it is distinct (for example CheckAccess() == false) before exercising the guard. This is the Bug: Bug: Breadcrumb thread-affinity tests assume Task.Run yields a distinct thread #900 pattern.
  • A file-handle test uses a stream the test owns, supplied through the wrapper's seam or an in-memory stream. It never uses a repository file.
  • Temporary files are prohibited by the unit-test policy and must not be used.

Actual Behavior

The guard under test can pass without being exercised, and the file-open test can fail or pass depending on MSBuild worker residency.

Logs / Screenshots

  • Attached minimal logs or screenshot
  • Snippet: none (static findings, verified present on 2026-09-28)

Impact / Severity

  • Blocker
  • High
  • Medium
  • Low

Source

From: docs/features/potential/2026-09-28-tests-depend-on-uncontrolled-environment.md

Activity

  1. drmoisan commented on Sep 28, 2026

    @drmoisan
    OwnerAuthor

    The following sections were present in the promoted potential entry (docs/features/potential/promoted/2026-09-28-tests-depend-on-uncontrolled-environment.md) but were dropped by potential_to_issue (defect #887, now consolidated into #932). Reposted verbatim.

    Suspected Cause / Notes

    Both patterns were copied from earlier tests and survived because they usually pass. Tests must run in parallel. Do not fix either defect with Workers=1, [DoNotParallelize] or retries.

    Proposed Fix / Validation Ideas

    • Apply the Bug: Bug: Breadcrumb thread-affinity tests assume Task.Run yields a distinct thread #900 dedicated-thread pattern at every triaged site. Split ItemViewerBreadcrumbThreadAffinityTests.cs so it stays at or under 500 lines, and register any new file in QuickFiler.Test.csproj.
    • Replace the TaskMaster.sln fixture with a test-owned stream or an injected seam. Add a seam to the wrapper only if one does not already exist.
    • Each rewritten test must be shown to fail against a deliberately broken guard, so the test demonstrably exercises the guard.
    • Run the full QuickFiler.Test and UtilitiesCS.Test suites in the parallel regime.

    Next Step

    • Promote to GitHub issue (bug-report template)
    • Move to active fix folder / branch

    Consolidates: #905, #906.

  2. drmoisan commented on Sep 30, 2026

    @drmoisan
    OwnerAuthor

    Delivered by PR 939 (merge commit ddbab26), parallel run bugs-2026-09-28. All 19 acceptance criteria in spec.md are checked off; CI green on head 8e3c3b5 (run 36651257792) before merge. Follow-ups named in the PR body are still to be filed separately.

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