Skip to content

fix(quickfiler-test): remove dead live-Form types from QuickFiler.Test and add a permanent structural guard (#491) - #588

Merged
drmoisan merged 7 commits into
epic/quickfiler-suite-determinism-foundation-integrationfrom
bug/quickfiler-test-form1-live-form-491-exec
Aug 22, 2026
Merged

fix(quickfiler-test): remove dead live-Form types from QuickFiler.Test and add a permanent structural guard (#491)#588
drmoisan merged 7 commits into
epic/quickfiler-suite-determinism-foundation-integrationfrom
bug/quickfiler-test-form1-live-form-491-exec

Conversation

@drmoisan

Copy link
Copy Markdown
Owner

Suggested title

fix(quickfiler-test): remove dead live-Form types from QuickFiler.Test and add a permanent structural guard (#491)

Summary

  • Deletes the dead QuickFiler.Test.Form1 type (Form1.cs, Form1.Designer.cs, Form1.resx) and its three compile/embedded-resource entries in QuickFiler.Test.csproj. No test constructed it; it was compiled but unused.
  • Adds a permanent reflection-only MSTest structural guard, NoLiveFormInTestAssemblyTests.ExecutingAssembly_ContainsNoFormDerivedType, that fails the build the next time any System.Windows.Forms.Form-derived type is compiled into QuickFiler.Test.
  • The new guard immediately caught a second, previously unknown, pre-existing dead type: a nested class QfcFormViewerDerived : QfcFormViewer in QuickFiler.Test/Controllers/QfcHomeControllerTests.cs, unrelated to Form1 and never instantiated anywhere in the repository (verified by repo-wide search). That dead class is also removed in this PR (remediation cycle 1).
  • A third, unrelated live-Form construction was found in a different assembly (UtilitiesCS.Test/ResourceTests.cs:20, Form1 frm = new Form1();) and is explicitly out of scope for this PR; it is tracked as its own issue, Bug: utilitiescs-test-form1-live-form #586.
  • Promotes issue Bug: quickfiler-test-form1-live-form #491's Item 2 (three test-only internal members of ItemViewer.Breadcrumb.cs) as an explicit tracking deferral to the ItemViewer-owning epic child, posted as a comment on Bug: quickfiler-test-form1-live-form #491.

Why

QuickFiler.Test/Form1.cs and Form1.Designer.cs declared a System.Windows.Forms.Form-derived type compiled into the unit-test assembly with no test referencing it. .claude/rules/general-unit-test.md and this epic's "never construct live forms in unit tests" invariant treat this as dead weight one new Form1() away from becoming a live policy violation. The fix removes the type and adds a guard that makes a future reintroduction fail the build immediately, rather than relying on manual review to catch it again.

During execution, the new guard test (which scopes to everyForm-derived type in the executing assembly, exactly as designed) also caught a second, unrelated dead type that predates this PR (QfcFormViewerDerived, first present 2025-02-01). It was deleted in a remediation cycle because it is the same defect class this PR exists to eliminate, has zero callers anywhere in the repository, and lives in the same assembly this PR's guard test targets, directly blocking that guard's acceptance criteria.

What Changed

Core change

  • Removed: QuickFiler.Test/Form1.cs, QuickFiler.Test/Form1.Designer.cs, QuickFiler.Test/Form1.resx.
  • Removed: nested dead class QfcFormViewerDerived in QuickFiler.Test/Controllers/QfcHomeControllerTests.cs (11 lines; no other change to that file).
  • Added: QuickFiler.Test/NoLiveFormInTestAssemblyTests.cs — reflection-only guard, scoped to the executing assembly only (does not flag the unrelated, legitimate production type QuickFiler.Viewers.Form1 in the referenced QuickFiler assembly).
  • QuickFiler.Test/QuickFiler.Test.csproj: confined edit to the two owned regions (Form1 compile entries, Form1.resx item group) — 1 line added, 11 lines deleted across 2 hunks. No other region of this 116-entry legacy project file was touched (three sibling epic children edit other regions of the same file concurrently).

Tests / tooling

  • No test framework, mocking library, or assertion library changes. New test uses MSTest + FluentAssertions per repo convention.

Docs

  • docs/features/active/2026-08-07-quickfiler-test-form1-live-form-491/spec.md: 10 of 11 acceptance criteria checked off with cited evidence (see Verification below for the 11th).
  • Full evidence trail under docs/features/active/2026-08-07-quickfiler-test-form1-live-form-491/evidence/ (baseline, regression-testing, qa-gates, issue-updates, other) plus policy-audit, code-review, and feature-audit artifacts from an independent feature-review pass.
  • New potential-bug promotion for the out-of-scope UtilitiesCS.Test/Form1 finding: issue Bug: utilitiescs-test-form1-live-form #586.

Architecture / How It Fits Together

NoLiveFormInTestAssemblyTests is a pure MSTest structural guard: it calls Assembly.GetExecutingAssembly(), enumerates loadable types via GetTypes() (guarding against ReflectionTypeLoadException so one unloadable unrelated type cannot leave the guard permanently red), and asserts via FluentAssertions that no type is assignable to System.Windows.Forms.Form. It constructs nothing — no form, no control, no BackgroundWorker — so it introduces no new visible-window risk itself; it exists specifically to prevent one.

Verification

Completed (from evidence artifacts committed on this branch):

  • CSharpier format/check: 0 files needing formatting (post-remediation).
  • MSBuild analyzers rebuild (/p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true): exit 0, 0 errors, non-vacuous rebuild confirmed (CoreCompile invoked, Skipping target "CoreCompile" count 0).
  • MSBuild nullable-as-errors rebuild (/p:TreatWarningsAsErrors=true, no /p:Nullable=enable): exit 0, 0 errors.
  • Full-suite vstest with /EnableCodeCoverage /InIsolation /TestCaseFilter:"TestCategory!=LiveOutlook": baseline 6437 total / 6436 passed / 1 failed (pre-existing unrelated flaky STA-timing test) / 0 skipped -> post-change 6438 total / 6438 passed / 0 failed / 0 skipped. No pre-existing test regressed; the one new guard test is the only addition, and it is green.
  • Guard test demonstrated red-then-green twice: against Form1 (primary cycle) and against QfcFormViewerDerived (remediation cycle), proving it is load-bearing rather than vacuous.
  • Coverage: baseline 85.5788% (lines-covered=53402, lines-valid=62401) vs. post-change 85.5627% (lines-covered=53392, lines-valid=62401) — a 10-line / 0.0161-percentage-point shortfall. Independently re-derived by feature-review directly from the committed Cobertura XML files: this PR's own diff has a measured coverage effect of exactly zero (no QuickFiler.Test class appears in either coverage file, consistent with the harness's .Test-assembly exclusion), and the shortfall is attributable to two specific, unrelated, untouched production files (UtilitiesCS/HelperClasses/SegmentStopWatch.cs, UtilitiesCS/OutlookObjects/Table/OlTableExtensions.Etl.cs). Both readings individually clear the repository's 85% floor. This is the one acceptance criterion (of 11) left unchecked in spec.md; feature-review dispositioned it non-blocking on this independently verified evidence.
  • Independent feature-review (policy-audit, code-review, feature-audit): all three PASS, 0 Blocking findings across all three artifacts.

Recommended (for a reviewer who wants to re-run locally):

  • dotnet tool run csharpier check .
  • msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true
  • msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:TreatWarningsAsErrors=true
  • vstest.console.exe QuickFiler.Test\bin\Debug\QuickFiler.Test.dll /EnableCodeCoverage /InIsolation /TestCaseFilter:"TestCategory!=LiveOutlook&FullyQualifiedName~NoLiveFormInTestAssemblyTests"

Not verified in this PR: this epic's PRs against the integration branch receive no automated CI checks (.github/workflows/ci.yml triggers only on main/development), so the verification above is the full local toolchain, not a CI run.

Backward Compatibility / Migration Notes

No public API changes. QuickFiler.Test.Form1 and QuickFiler.Test.Form1.Designer are removed; nothing outside their own two files referenced them (verified by repo-wide search before removal). No breaking change to any production assembly.

Risks and Mitigations

  • Risk: the new guard test could be too broad and flag a legitimate future test double that extends a Form-derived type without ever showing it (as QfcFormViewerDerived did, though unused). Mitigation: the guard was deliberately left broad rather than narrowed with an exemption list, per repository convention of fixing dead code rather than special-casing coverage/policy gates; a future legitimate case would need its own design decision, not a silent allowlist.
  • Risk: coverage read a small amount below baseline. Mitigation: independently re-verified as measurement noise from unrelated, untouched files with zero contribution from this PR's own diff; documented in detail in feature-audit.2026-08-22T15-26.md.

Review Guide

Suggested order:

  1. QuickFiler.Test/NoLiveFormInTestAssemblyTests.cs — the new guard, small and self-contained.
  2. QuickFiler.Test/QuickFiler.Test.csproj diff — confirm the edit is confined to the two owned regions.
  3. QuickFiler.Test/Controllers/QfcHomeControllerTests.cs diff — confirm only the dead nested class was removed (11 lines, no other change).
  4. docs/features/active/2026-08-07-quickfiler-test-form1-live-form-491/spec.md and the three review artifacts (policy-audit, code-review, feature-audit) for the full acceptance-criteria and audit trail.

Noisy/mechanical: the committed raw Cobertura XML evidence files (coverage-baseline.cobertura.xml, coverage-postchange-remediation.cobertura.xml) are large and can be skipped in review; their headline numbers are summarized in the QA-gate .md artifacts alongside them.

Follow-ups

  • Issue Bug: utilitiescs-test-form1-live-form #586 (new): remove or fix the live Form1 construction in UtilitiesCS.Test/ResourceTests.cs:20, in a different assembly, out of scope for this PR.
  • Item 2 of the original potential document (three test-only internal members of QuickFiler/Viewers/ItemViewer.Breadcrumb.cs) remains deferred to the ItemViewer-owning epic child; tracked via a comment on Bug: quickfiler-test-form1-live-form #491.
  • Optional: a maintainer may choose to re-capture coverage in a quieter session to obtain a numerically-passing reading for the one unchecked acceptance criterion, though feature-review has already dispositioned it non-blocking.

GitHub Auto-close

@drmoisan
drmoisan merged commit b8b4a9e into epic/quickfiler-suite-determinism-foundation-integrationAug 22, 2026
drmoisan added a commit that referenced this pull request Aug 22, 2026
Children 449 (PR #585, follow-up #590), 445 (PR #587), and 491 (PR #588)
are merged; each merge commit was confirmed reachable from the fetched
integration head rather than taken from a completion notification. Child
511 remains in atomic execution.
Records seven carried findings, two of which correct this epic's own
inputs: epic.md misattributed QuickFiler/Legacy/QuickFileController.cs's
1,065 lines to QuickFiler/Controllers/QfcExplorerController.cs (182 lines
after change, and the legacy file has zero compile references), and
collect_pr_context writes into the shared main checkout, letting one
child overwrite a sibling's PR context (issue #589).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UHj7wjLweuwfAP8NDA4iiP
Sign up for freeto 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

@drmoisan