Fix RealDataTestsBase cross-worktree test collisions#979
Conversation
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>
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>
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
left a comment
There was a problem hiding this comment.
@jasonleenaylor reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on johnml1135).
jasonleenaylor
left a comment
There was a problem hiding this comment.
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: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.
Summary
RealDataTestsBase(backingRenderBaselineTests,RenderBenchmarkTestsBase, and subclasses) resolved its scratch test-project directory viaFwDirectoryFinder.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 physicalintegration_test_datafolder when running these tests.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.FwDirectoryFinder's production API is untouched):DbDirectory()now resolves under this checkout's ownSourceDirectory/../DistFiles/Projects, matching the pattern already used by other real-cache test fixtures in this repo (ConfiguredXHTMLGeneratorTests,RecordListTests, etc.).SourceDirectory, becauseFwNewLangProjectModel.CheckForUniqueProjectName— correct, unrelated production logic for the real "New Project" wizard — checks name uniqueness against the sharedProjectsDirectoryregardless 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../test.ps1CI-equivalent run🤖 Generated with Claude Code
This change is