Skip to content

Fix RealDataTestsBase cross-worktree test collisions#979

Open
johnml1135 wants to merge 1 commit into
mainfrom
fix/real-data-tests-worktree-isolation
Open

Fix RealDataTestsBase cross-worktree test collisions#979
johnml1135 wants to merge 1 commit into
mainfrom
fix/real-data-tests-worktree-isolation

Conversation

@johnml1135

@johnml1135 johnml1135 commented Jul 1, 2026 •
edited by jasonleenaylor
Loading

Copy link
Copy Markdown
Contributor

Summary

  • RealDataTestsBase (backing RenderBaselineTests, RenderBenchmarkTestsBase, and subclasses) resolved its scratch test-project directory via FwDirectoryFinder.ProjectsDirectory, a per-machine registry value that's correct for the shipping app (one shared Projects folder machine-wide) but means every git worktree of this repo collides on the same physical integration_test_data folder when running these tests.
  • Symptom: System.InvalidOperationException: Refusing to delete '...\integration_test_data' because the test sentinel file '.fieldworks-real-data-test-project' is missing — one worktree's test run deletes/rewrites another's project mid-run.
  • Fix (scoped entirely to this test fixture, FwDirectoryFinder's production API is untouched):
    1. DbDirectory() now resolves under this checkout's own SourceDirectory/../DistFiles/Projects, matching the pattern already used by other real-cache test fixtures in this repo (ConfiguredXHTMLGeneratorTests, RecordListTests, etc.).
    2. The project name is suffixed with a short hash of the worktree's SourceDirectory, because FwNewLangProjectModel.CheckForUniqueProjectName — correct, unrelated production logic for the real "New Project" wizard — checks name uniqueness against the shared ProjectsDirectory regardless of where this fixture's files live, so a fixed name still collided across worktrees. The suffix is stable within a worktree (preserving the mutex-guarded "reusable project" perf intent) but unique across worktrees.

Found and fixed while running CI locally as part of an unrelated PR #964 review — see that PR's PR_964_review.md §15 for the full investigation trail.

Test plan

  • RenderBaselineTests + RealDataTestsBaseCleanupTests (9 tests): went from 8/9 failing with the collision error to 9/9 passing.
  • Full ./test.ps1 CI-equivalent run

🤖 Generated with Claude Code

This change is Reviewable

@johnml1135 @claude
Fix RealDataTestsBase cross-worktree test collisions
7a61c4f 
RealDataTestsBase (used by RenderBaselineTests/RenderBenchmarkTestsBase and
their subclasses) resolved its scratch project directory via
FwDirectoryFinder.ProjectsDirectory, which reads a per-machine registry
value (correct for the shipping app - one shared Projects folder regardless
of which install launches it, by design). For this test fixture, that's a
liability: every git worktree of this repo shares the same registry value,
so two worktrees running these tests collide on the same physical
"integration_test_data" folder - one worktree's TestSetup/TestTearDown can
delete or rewrite a project mid-run in another, tripping the sentinel-file
safety check ("Refusing to delete ... because the test sentinel file
'.fieldworks-real-data-test-project' is missing").

Two changes, both scoped to this test fixture only (FwDirectoryFinder's
production API is untouched):

1. DbDirectory() now resolves under this checkout's own
   SourceDirectory/../DistFiles/Projects instead of the shared,
   registry-resolved ProjectsDirectory - the same pattern already used by
   several other real-cache test fixtures in this repo (e.g.
   ConfiguredXHTMLGeneratorTests, RecordListTests).

2. The project name itself is now suffixed with a short hash of this
   worktree's SourceDirectory. This is needed because
   FwNewLangProjectModel.CheckForUniqueProjectName - correct, unrelated
   production logic used by the real "New Project" wizard - checks name
   uniqueness against the shared ProjectsDirectory regardless of where this
   fixture's own files live, so a fixed name would still collide with
   another worktree's project of the same name. The suffix is stable across
   repeated runs within the same worktree (preserving the mutex-guarded
   "reusable project" performance intent) while guaranteeing no collision
   across worktrees.

Verified: RenderBaselineTests + RealDataTestsBaseCleanupTests (9 tests) go
from 8 failing with "Refusing to delete..." to 9/9 passing.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

NUnit Tests

    1 files  ±0      1 suites  ±0   10m 32s ⏱️ +28s
4 299 tests ±0  4 226 ✅ ±0  73 💤 ±0  0 ❌ ±0 
4 308 runs  ±0  4 235 ✅ ±0  73 💤 ±0  0 ❌ ±0 

Results for commit 7a61c4f. ± Comparison against base commit fb4c99d.

@johnml1135 johnml1135 mentioned this pull request Jul 1, 2026
Draft
johnml1135 added a commit that referenced this pull request Jul 1, 2026
@johnml1135 @claude
Phase-1 base: Avalonia migration spine (UIMode defaults Legacy)
af20c5b 
First of a 4-PR stack landing Phase 1 of the WinForms->Avalonia migration.
Every Avalonia surface is gated behind the UIMode setting, which DEFAULTS TO
"Legacy" (Src/Common/FwUtils/Properties/Settings.Designer.cs), so default
users see no behavioral change.

Contents:
- The Avalonia migration framework: region/composer (FullEntryRegionComposer),
  the typed view-definition IR compiled from XML layouts, owned dense controls,
  the seam contracts, and the region-editor plugin registry.
- The base detail-editor surfaces active under UIMode=New: lexiconEdit,
  lexiconEditPopup, notebookEdit, posEdit.
- The Avalonia browse table (LexicalBrowseView, LexicalBrowseHostControl,
  BulkEditBarView, ClerkBrowse*) and its RecordBrowseView product wiring,
  shipped DORMANT (fused to base wiring; activated by the table follow-up PR).
- 13 dialog UIs + a shared MessageBox riding behind UIMode=New, each verified
  wired to a real WinForms call site; 4 cleanly-removable unwired Avalonia
  dialogs backed out (SpecialCharacter, WritingSystemProperties,
  DeleteConfirmation, LexReferenceDetails).
- ChorusNotesBarControl (the Chorus/FLExBridge notes bar) rides as a Phase-1
  follow-up surface alongside the browse table and the interlinear/
  rule-formula plugins -- built, but not registered in this base PR.
- Migration skills/playbook (incl. the inert-surface activation recipe), the
  roadmap's core proposal/design/tasks/spec, and the landed openspec change
  specs (lexical-edit-avalonia-migration, shared-editable-virtualized-table,
  avalonia-multi-writing-system-text-foundation).

The interlinear and rule-formula detail editors are carved out to their own
stacked follow-up PRs; the inert tool lists are
LexicalEditSurfaceRegistry.Phase1FollowUpSurfaceTools and
LexicalEditSurfaceResolver.Phase1FollowUpBrowseTools. Migration per-screen
docs (the JIRA-ticket basis) and the full 13-stage migration-program planning
material live on the separate never-merged phase1-docs branch (the latter
also mirrored to chore/relocate-roadmap-planning-docs).

Phase 2 (avalonia-end-game: net multiplatform + shell conversion + WinForms
removal) is planned only and gated on Phase 1 + tester burn-down; its
proposal lives with the roadmap material above, not in this PR.

Post-review cleanup folded into this commit (see PR_964_review.md for the
full audit trail):
- Removed content unrelated to this migration that had been swept in:
  an unrelated installer-docs deletion and an ungated Legacy-mode rendering
  change were reverted, and a misplaced/stale stray doc file was dropped.
- Split two unrelated-but-legitimate fixes into their own PRs rather than
  carrying them here: #978 (VersionInfoProvider copyright/version-string
  bugs, a stale RegFree.targets entry, an opsx-prompt refactor) and #979
  (a pre-existing cross-worktree test-collision bug in RealDataTestsBase,
  found while verifying this branch but unrelated to it).
- Fixed a real build break (RecordBrowseView.cs used a pub/sub API signature
  main had already replaced) and a real product bug (the Avalonia refresh
  controller could stay unwired when a tool loads directly into UIMode=New,
  fixed in RecordEditView.cs).
- Pared back ~7,500 lines of speculative future-phase openspec planning
  docs (the 13-stage migration program, legacy-screenshot-capture tooling,
  the end-game proposal) to a separate branch, and deleted two fully
  superseded proposals and stale DataTree-model-view-separation specs
  asserting an architecture that was never built.
- Wired the one missing UIMode gate (GoLinkEntryDlgListener.OnGotoLexEntry),
  documented one honest parity deferral (MsaInflectionFeatureListDlgLauncher
  switch-tools navigation), removed dead API and orphaned localization keys,
  strengthened the FwAvalonia engine-isolation audit to match its own
  documented symbol list, fixed three evidence-language issues in
  Path3BundleTests, and reconciled dialog spacing tokens against real
  measured WinForms control geometry.

Verification: whole-solution build green. Surface-registry census: 6
custom-slice classes classified (LexemeEditorBurnDownTests); 7 tools in
LexicalEditSurfaceRegistry.Phase1FollowUpSurfaceTools; 8 tools in
LexicalEditSurfaceResolver.Phase1FollowUpBrowseTools. Full CI-equivalent test
run: all tests pass except 38 pre-existing, environment-specific
RealDataTestsBase cross-worktree-collision failures (fix is PR #979, kept
separate) and one test-harness limitation in RecordEditViewSwitchTests
documented in PR_964_review.md (the underlying product bug is fixed; the
test's idle-queue draining has a separate, deeper, pre-existing issue that
needs its own follow-up).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
johnml1135 added a commit that referenced this pull request Jul 2, 2026
@johnml1135 @claude
Phase-1 base: Avalonia migration spine (UIMode defaults Legacy)
d9fbe71 
First of a 4-PR stack landing Phase 1 of the WinForms->Avalonia migration.
Every Avalonia surface is gated behind the UIMode setting, which DEFAULTS TO
"Legacy" (Src/Common/FwUtils/Properties/Settings.Designer.cs), so default
users see no behavioral change.

Contents:
- The Avalonia migration framework: region/composer (FullEntryRegionComposer),
  the typed view-definition IR compiled from XML layouts, owned dense controls,
  the seam contracts, and the region-editor plugin registry.
- The base detail-editor surfaces active under UIMode=New: lexiconEdit,
  lexiconEditPopup, notebookEdit, posEdit.
- The Avalonia browse table (LexicalBrowseView, LexicalBrowseHostControl,
  BulkEditBarView, ClerkBrowse*) and its RecordBrowseView product wiring,
  shipped DORMANT (fused to base wiring; activated by the table follow-up PR).
- 13 dialog UIs + a shared MessageBox riding behind UIMode=New, each verified
  wired to a real WinForms call site; 4 cleanly-removable unwired Avalonia
  dialogs backed out (SpecialCharacter, WritingSystemProperties,
  DeleteConfirmation, LexReferenceDetails).
- ChorusNotesBarControl (the Chorus/FLExBridge notes bar) rides as a Phase-1
  follow-up surface alongside the browse table and the interlinear/
  rule-formula plugins -- built, but not registered in this base PR.
- Migration skills/playbook (incl. the inert-surface activation recipe), the
  roadmap's core proposal/design/tasks/spec, and the landed openspec change
  specs (lexical-edit-avalonia-migration, shared-editable-virtualized-table,
  avalonia-multi-writing-system-text-foundation).

The interlinear and rule-formula detail editors are carved out to their own
stacked follow-up PRs; the inert tool lists are
LexicalEditSurfaceRegistry.Phase1FollowUpSurfaceTools and
LexicalEditSurfaceResolver.Phase1FollowUpBrowseTools. Migration per-screen
docs (the JIRA-ticket basis) and the full 13-stage migration-program planning
material live on the separate never-merged phase1-docs branch (the latter
also mirrored to chore/relocate-roadmap-planning-docs).

Phase 2 (avalonia-end-game: net multiplatform + shell conversion + WinForms
removal) is planned only and gated on Phase 1 + tester burn-down; its
proposal lives with the roadmap material above, not in this PR.

Post-review cleanup folded into this commit (see PR_964_review.md for the
full audit trail):
- Removed content unrelated to this migration that had been swept in:
  an unrelated installer-docs deletion and an ungated Legacy-mode rendering
  change were reverted, and a misplaced/stale stray doc file was dropped.
- Split two unrelated-but-legitimate fixes into their own PRs rather than
  carrying them here: #978 (VersionInfoProvider copyright/version-string
  bugs, a stale RegFree.targets entry, an opsx-prompt refactor) and #979
  (a pre-existing cross-worktree test-collision bug in RealDataTestsBase,
  found while verifying this branch but unrelated to it).
- Fixed a real build break (RecordBrowseView.cs used a pub/sub API signature
  main had already replaced) and a real product bug (the Avalonia refresh
  controller could stay unwired when a tool loads directly into UIMode=New,
  fixed in RecordEditView.cs).
- Pared back ~7,500 lines of speculative future-phase openspec planning
  docs (the 13-stage migration program, legacy-screenshot-capture tooling,
  the end-game proposal) to a separate branch, and deleted two fully
  superseded proposals and stale DataTree-model-view-separation specs
  asserting an architecture that was never built.
- Wired the one missing UIMode gate (GoLinkEntryDlgListener.OnGotoLexEntry),
  documented one honest parity deferral (MsaInflectionFeatureListDlgLauncher
  switch-tools navigation), removed dead API and orphaned localization keys,
  strengthened the FwAvalonia engine-isolation audit to match its own
  documented symbol list, fixed three evidence-language issues in
  Path3BundleTests, and reconciled dialog spacing tokens against real
  measured WinForms control geometry.

Verification: whole-solution build green. Surface-registry census: 6
custom-slice classes classified (LexemeEditorBurnDownTests); 7 tools in
LexicalEditSurfaceRegistry.Phase1FollowUpSurfaceTools; 8 tools in
LexicalEditSurfaceResolver.Phase1FollowUpBrowseTools. Full CI-equivalent test
run: all tests pass except 38 pre-existing, environment-specific
RealDataTestsBase cross-worktree-collision failures (fix is PR #979, kept
separate) and one test-harness limitation in RecordEditViewSwitchTests
documented in PR_964_review.md (the underlying product bug is fixed; the
test's idle-queue draining has a separate, deeper, pre-existing issue that
needs its own follow-up).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
jasonleenaylor
jasonleenaylor approved these changes Jul 2, 2026
View reviewed changes

@jasonleenaylor jasonleenaylor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@jasonleenaylor reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on johnml1135).

jasonleenaylor
jasonleenaylor reviewed Jul 2, 2026
View reviewed changes

@jasonleenaylor jasonleenaylor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A commit comment that contains all the comments also present in the code is a bit much. Please make sure we commit a much more results focused comment rather than details of the solution and maybe shorten the code comments as well.

@jasonleenaylor made 2 comments.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on johnml1135).

Src/Common/RootSite/RootSiteTests/RealDataTestsBase.cs line 169 at r1 (raw file):

			// delete or rewrite another's mid-run). Anchor to this checkout's own DistFiles/Projects
			// instead - the same SourceDirectory-derived, registry-free pattern already used by other
			// real-cache test fixtures in this repo (e.g. ConfiguredXHTMLGeneratorTests, RecordListTests).

This comment is awefully long and wordy, wouldn't mind a pass at tightening it up.

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

Reviewers

@jasonleenaylor jasonleenaylor jasonleenaylor approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@johnml1135 @jasonleenaylor