All tests pass

Full analysis of all test changes

Author: johnml1135

.gitignore

@@ -178,6 +178,10 @@ CrashLog.txt
 TestResults/
 *.trx
 
+# WPF markup compilation temp projects
+*_wpftmp.csproj
+*_wpftmp.vbproj
+
 # WiX v6
 *.wixpdb
 

Src/FwCoreDlgs/FindCollectorEnv.cs

@@ -44,9 +44,9 @@ public class FindCollectorEnv : CollectorEnv, IDisposable
 		/// <summary></summary>
 		protected IVwSearchKiller m_searchKiller;
 		/// <summary>
-		/// True once we've reached the start location at least once during the current find.
-		/// This allows us to apply the start offset across duplicate renderings of the same
-		/// underlying string property (same hvo+tag but different cpropPrev).
+		/// True once we've reached the start location during the current find.
+		/// Once true, we should stop filtering objects (DisplayThisObject) so traversal
+		/// can continue normally beyond the start point.
 		/// </summary>
 		private bool m_fReachedStartLocation;
 		#endregion
@@ -112,17 +112,18 @@ public LocationInfo FindNext(LocationInfo startLocation)
 
 		private bool StartLocationAppliesToCurrentProp(int tag)
 		{
-			return m_StartLocation != null && m_StartLocation.TopLevelHvo == m_hvoCurr && m_StartLocation.m_tag == tag;
+			// Apply the start offset only to the exact occurrence of the string property the
+			// selection is in (tag + cpropPrev). Subsequent occurrences (different cpropPrev)
+			// should search from the start of their own string.
+			return m_StartLocation != null
+				&& m_StartLocation.TopLevelHvo == m_hvoCurr
+				&& m_StartLocation.m_tag == tag
+				&& m_StartLocation.m_cpropPrev == CPropPrev(tag);
 		}
 
 		/// <summary>
 		/// Returns true if we should skip searching this property because we haven't reached the
 		/// start location yet.
-		///
-		/// Once we have reached the start location, we keep <see cref="m_StartLocation"/> in
-		/// place for any duplicate renderings of the same underlying string property (same hvo+tag),
-		/// so they all respect the start offset and we don't re-find earlier matches in later
-		/// duplicate instances.
 		/// </summary>
 		private bool ShouldSkipBecauseStartNotReached(int tag)
 		{
@@ -164,7 +165,7 @@ private bool ShouldSkipBecauseStartNotReached(int tag)
 		protected override bool DisplayThisObject(int hvoItem, int tag)
 		{
 			// We want to skip the beginning until we reach our start location.
-			if (m_StartLocation == null || Finished)
+			if (m_StartLocation == null || Finished || m_fReachedStartLocation)
 				return true;
 
 			int cPropPrev = CPropPrev(tag);
@@ -223,8 +224,6 @@ public override void AddString(ITsString tss)
 				// the tag that belongs to the open property. Some VCs output the contents of
 				// a string property via AddString (e.g., around ORCs). We need to search those
 				// strings too, not just AddStringProp/AddStringAltMember.
-				CheckForStartLocationAndLimit(m_tagCurrent);
-
 				if (ShouldSkipBecauseStartNotReached(m_tagCurrent))
 					return;
 
@@ -440,7 +439,7 @@ protected virtual void DoFind(ITsString tss, int tag)
 
 			IVwTextSource textSource = m_textSourceInit as IVwTextSource;
 			int ichBegin = 0;
-			if (m_StartLocation != null)
+			if (StartLocationAppliesToCurrentProp(tag))
 			{
 				Debug.Assert(m_StartLocation.TopLevelHvo == m_hvoCurr && m_StartLocation.m_tag == tag);
 				ichBegin = m_StartLocation.m_ichLim;

TEST_CHANGE_LEDGER.md

@@ -2,6 +2,8 @@
 
 Range: `3c2e41b9..HEAD`
 
+Note: the working tree currently includes additional, uncommitted changes (not represented in the git range above). Those are called out inline where relevant.
+
 Purpose: An exhaustive ledger of every commit in-range that touched `**/*Tests.cs`, with a per-file classification:
 
 - **Mechanical**: refactors/rewrites to keep tests compiling/running under new tooling (NUnit API changes, adapter-friendly asserts, mock framework swaps, formatting/cleanup) without materially changing what the test is trying to prove.
@@ -50,11 +52,16 @@ Supporting non-test `.cs` changes in this commit:
 - [B4] Src/Common/Controls/DetailControls/Slice.cs — **Substantive** — changes default expansion behavior so slices only auto-expand when explicitly marked `expansion="expanded"` (reduces UI-state surprises in tests).
 - [B1] Src/Common/FieldWorks/ProjectId.cs — **Substantive** — improves relative-path handling so paths with a directory component resolve under ProjectsDirectory without the “double nest” behavior.
 - [B2] Src/Common/SimpleRootSite/SimpleRootSite.cs — **Substantive** — best-effort unregisters `IVwNotifyChange` notifications during close/dispose to prevent post-dispose `PropChanged` callbacks destabilizing tests.
-- [B4] Src/FwCoreDlgs/FindCollectorEnv.cs — **Substantive** — fixes start-location semantics across duplicate renderings and captures WS at match offsets (stabilizes find/replace selection reconstruction).
+- [B4] Src/FwCoreDlgs/FindCollectorEnv.cs — **Substantive** — fixes start-location semantics across duplicate renderings, ensures the start offset applies only to the exact selected occurrence (tag + `cpropPrev`) including strings emitted via `AddString(...)`, and captures WS at match offsets (stabilizes find/replace selection reconstruction).
+
+Working tree follow-up (uncommitted):
+
+- [B4] Src/FwCoreDlgs/FindCollectorEnv.cs — **Substantive** — further refines start-location application to require an exact occurrence match and avoids clearing the start location prematurely in the `AddString(...)` path; resolves the remaining `FwFindReplaceDlgTests` failures under VSTest.
 - [B4] Src/FwCoreDlgs/FwFindReplaceDlg.cs — **Substantive** — TE-1658: WS-only replace behavior (empty Find text + Match WS) no longer advances incorrectly; adds selection restoration + WS propagation.
 - [B9] Src/xWorks/CssGenerator.cs — **Substantive** — avoids ExCSS crashes on `inherit` by using `none`/`unset` (and correct `UnitType.Ident`) to keep CSS generation deterministic.
 - [C2] Src/xWorks/DictionaryConfigManager.cs — **Substantive** — keeps rename operation defensive (revert/refresh on protected items) to match test expectations.
 - [D1] Build/Src/FwBuildTasks/RegFreeCreator.cs — **Mechanical** — fixes a node selection bug when adding/replacing CLR class entries in reg-free manifests.
+- [D1] test.ps1 — **Mechanical** — mitigates a VSTest multi-assembly edge case where `vstest.console.exe` returns exit code `-1` even when TRX reports 0 failures, by retrying per-assembly and aggregating exit codes; emits per-assembly TRX + console logs to aid isolation.
 
 ### 2025-12-17 — `44e740104481e4a0c2d9cdda2dc615931b4e9fbf` — Clear ignore fixes
 

TEST_UPDATES_FROM_MODERNIZATION.md

@@ -74,7 +74,7 @@ For the complete classification of “baseline ignore was removed/reintroduced
   - In [TEST_CHANGE_LEDGER.md](TEST_CHANGE_LEDGER.md) under commit `65f81dc…`, WinForms-heavy fixtures (e.g., `StVcTests`, `FwFindReplaceDlgTests`) were moved toward STA and explicit layout/root creation.
   - In the same commit, the underlying find/replace implementation was adjusted (`FindCollectorEnv`, `FwFindReplaceDlg`) to make selection reconstruction and WS-only search/replace behavior more deterministic under isolation.
   - UI-state defaults that influence tests were also tightened (e.g., `Slice` expansion now only auto-expands when explicitly marked).
-  - Note: several `FwFindReplaceDlgTests` still fail today; those are tracked in the TODO section below.
+  - Follow-up: `FwFindReplaceDlgTests` are now passing under VSTest after refining `FindCollectorEnv` start-location semantics for the `AddString(...)` rendering path and ensuring the start offset only applies to the exact selected occurrence of the string property.
 
 #### B5) ICU data discovery (ICU_DATA)
 
@@ -147,6 +147,18 @@ For the complete classification of “baseline ignore was removed/reintroduced
 - Why this is treated as mechanical:
   - These changes generally don’t alter product behavior; they’re required to keep tests buildable/runnable and reporting correctly under the modern runner.
 
+#### D1) VSTest multi-assembly exit code `-1` (false fail)
+
+- What we observed:
+  - A full-suite run that invoked `vstest.console.exe` with multiple test assemblies returned exit code `-1`, causing `test.ps1` to report `[FAIL] ... exit code: -1`.
+  - The generated TRX indicated `failed="0"` (i.e., no failing tests), suggesting the exit code was unreliable for that scenario.
+- Why modernization surfaced it:
+  - Moving from prior runners to VSTest changes process topology (testhost/execution isolation) and how failures are reported. In practice, “multi-assembly + VSTest” occasionally yields a non-specific `-1` even without test failures.
+- Fix rationale (root cause):
+  - We treat the TRX/test results as the source of truth and make the orchestration resilient: when the multi-assembly invocation returns `-1`, `test.ps1` retries per assembly and aggregates exit codes.
+  - This preserves strictness (real failures still fail the run) while avoiding false negatives caused by a runner edge case.
+  - The per-assembly retry also produces actionable evidence: per-assembly TRX (`<AssemblyName>_<timestamp>.trx`) and per-assembly console logs (`vstest.<AssemblyName>.console.log`) in `Output\<Configuration>\TestResults`.
+
 ## Fishy / uncertain mappings (worth a quick human review)
 
 Nothing here looks *obviously* off-scope for “stabilize tests under VSTest”, but these items do touch behavior enough that it’s reasonable to sanity-check intent:
@@ -171,21 +183,11 @@ Nothing here looks *obviously* off-scope for “stabilize tests under VSTest”,
 
 ## Current failing tests (TODO)
 
-As of TRX: `Output\Debug\TestResults\johnm_SIL-XPS_2025-12-19_10_18_05.trx` (see `Output\Debug\TestResults\vstest.console.log`), the failing set is:
-
-- `FwFindReplaceDlgTests.FindNextFromWithinMatchingWord` — selection verify mismatch (expected 1, was 0)
-- `FwFindReplaceDlgTests.FindNextWithMatch` — expected True, was False
-- `FwFindReplaceDlgTests.FindNextWithNoMatchAfterWrap` — expected True, was False
-- `FwFindReplaceDlgTests.FindWithMatchCase` — expected True, was False
-- `FwFindReplaceDlgTests.FindWithMatchDiacritics` — selection verify mismatch (expected 1, was 0)
-- `FwFindReplaceDlgTests.InitialFindPrevWithMatch` — selection verify mismatch (expected 2, was 0)
-- `FwFindReplaceDlgTests.InitialFindPrevWithMatchAfterWrap` — selection verify mismatch (expected 2, was 0)
-- `FwFindReplaceDlgTests.ReplaceTextAfterFootnote` — TsString mismatch (expected `BlaQlah!`, actual `Blah, blah, blah!`)
-- `FwFindReplaceDlgTests.ReplaceWithMatchWs_EmptyFindText` — selection verify mismatch (expected 3, was 7)
-
-Suggested next debugging focus:
-- Re-check `DummyFwFindReplaceDlg.VerifySelection(...)` invariants under VSTest isolation.
-- Determine whether failure is caused by selection reconstruction, wrap state, or view initialization ordering in the dummy view.
+As of the most recent TRX in `Output\Debug\TestResults` (see `Output\Debug\TestResults\vstest.console.log`), there are no remaining failures in `FwFindReplaceDlgTests`.
+
+Note: if a full-suite run ever reports `[FAIL] ... exit code: -1` while TRX shows 0 failures, `test.ps1` will retry per-assembly and the per-assembly TRX/logs should be used to confirm whether there are any real failures.
+
+Baseline-ignored tests remain ignored (per branch policy).
 
 ## “No popups ever” plan (with external citations)
 

test.ps1

@@ -460,6 +460,58 @@ try {
                 throw "Detected possible file is locked during vstest execution."
             }
         }
+
+        # =============================================================================
+        # Workaround: multi-assembly VSTest may fail with exit code -1 and minimal output
+        # =============================================================================
+
+        if (-not $ListTests -and $testDlls.Count -gt 1 -and $script:testExitCode -eq -1) {
+            Write-Host "[WARN] vstest.console.exe returned exit code -1 with multiple test assemblies. Retrying per-assembly to isolate failures." -ForegroundColor Yellow
+
+            $timestamp = Get-Date -Format 'yyyyMMdd_HHmmss'
+            $overallExitCode = 0
+
+            foreach ($testDll in $testDlls) {
+                $dllName = [System.IO.Path]::GetFileNameWithoutExtension($testDll)
+                Write-Host ""
+                Write-Host "Running tests in $dllName..." -ForegroundColor Cyan
+
+                $singleArgs = @()
+                $singleArgs += $testDll
+                $singleArgs += "/Platform:x64"
+                $singleArgs += "/Settings:$runSettingsPath"
+                $singleArgs += "/ResultsDirectory:$resultsDir"
+                $singleArgs += "/Logger:trx;LogFileName=${dllName}_${timestamp}.trx"
+                $singleArgs += "/Logger:console;verbosity=$vstestVerbosity"
+
+                if ($TestFilter) {
+                    $singleArgs += "/TestCaseFilter:$TestFilter"
+                }
+
+                $singleOutput = & $vstestPath $singleArgs 2>&1 | Tee-Object -Variable singleTestOutput
+                $singleExitCode = $LASTEXITCODE
+                if ($singleExitCode -ne 0 -and $overallExitCode -eq 0) {
+                    $overallExitCode = $singleExitCode
+                }
+
+                $singleLogPath = Join-Path $resultsDir "vstest.${dllName}.console.log"
+                try {
+                    $singleTestOutput | Out-File -FilePath $singleLogPath -Encoding UTF8
+                }
+                catch {
+                    Write-Host "[WARN] Failed to write VSTest output log to $singleLogPath" -ForegroundColor Yellow
+                }
+
+                if ($singleExitCode -ne 0) {
+                    $singleOutputText = ($singleTestOutput | Out-String)
+                    if ($singleOutputText -match 'used by another process|file is locked|cannot access the file') {
+                        throw "Detected possible file is locked during vstest execution."
+                    }
+                }
+            }
+
+            $script:testExitCode = $overallExitCode
+        }
     }
 }
 finally {