Skip to content

Commit ddf5015

Browse files
committed
Stabilize browser test isolation and route bootstrap
1 parent 7983efe commit ddf5015

54 files changed

Lines changed: 908 additions & 326 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

AGENTS.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ Rule format:
116116
- Every runnable test project must declare `MaxParallelTestsForPipeline : EnvironmentAwareParallelLimitBase` and rely on the base class limits by default; do not add per-suite `LocalLimit` or `CiLimit` overrides unless a specific suite has a documented exception.
117117
- Browser-suite CI parallelism is user-tunable. When the user wants higher throughput, keep in-suite browser-test CI parallelism in the `6-9` worker range when the harness can sustain it, and prefer extra GitHub Actions job splitting on top of that before reaching for timeout increases; do not pin browser suites back to `2` workers as a blanket fix.
118118
- Browser UI tests must be data-isolated and storage-isolated by default: each scenario should create or import its own script or workspace data, avoid shared persisted document IDs or browser-local state, and remain parallel-safe when multiple pages or contexts hit the app at the same time.
119-
- Local regression verification must include solution-level `dotnet test --solution ./PrompterOne.slnx --max-parallel-test-modules 1` so test-project split changes are proven under the real all-tests entrypoint, not only as isolated per-project runs.
119+
- Local regression verification must include a solution-level `dotnet test --solution ./PrompterOne.slnx` run so test-project split changes are proven under the real all-tests entrypoint with normal module parallelism; only force `--max-parallel-test-modules 1` when diagnosing a known module-level interference bug.
120120
- When the user explicitly asks to validate a test fix in actual GitHub Actions, do not spend more time on local `CI=true` emulation; push the fix and monitor the real CI run instead.
121121
- Do not use local `CI=true` browser-suite runs as a primary reproduction or validation signal unless the user explicitly asks for that exact experiment; in this repo it mostly changes in-suite parallelism and can hide the real CI failure mode instead of reproducing it.
122122
- When diagnosing browser-suite CI failures, always inspect and preserve Playwright screenshot artifacts from the failing run, and extend the harness so early bootstrap or fixture failures also leave a screenshot whenever the browser page still exists.
@@ -131,6 +131,7 @@ Rule format:
131131
- Repo-wide cleanup and review passes must explicitly inventory forbidden implementation string literals, `MarkupString` or raw-HTML UI composition, duplicated JS/CSS patterns, architecture-boundary drift, and `foreach`-driven test scenarios that should become isolated TUnit cases.
132132
- Repo-wide audits should use multiple independent reviewers with distinct focuses when the tooling is available, including external CLI reviewers such as Claude and Copilot plus internal agents, and all review outputs should be captured in root-level task files before remediation starts.
133133
- When CI browser suites keep flaking, use the available assistant CLIs and internal sub-agents as parallel investigators on the same failure cluster instead of debugging only through one serial line of inquiry.
134+
- Browser-suite stability fixes must prioritize per-test writable-state isolation and shared-harness correctness over retries, suite serialization, or other band-aids that only hide parallel interference.
134135
- Legacy, dead, duplicate, or speculative code paths should be deleted aggressively instead of being preserved behind compatibility instincts; if code has no clear runtime owner or authoritative contract, remove it rather than keep it as “just in case” ballast.
135136
- For repo-wide remediation passes, keep an explicit root-level accounting of fixed versus remaining feedback items, finish the code fixes first, and only then run and stabilize the test suites; do not bounce back into verification mid-remediation unless the user explicitly asks.
136137
- For task-scoped work, edit, stage, and commit only the files directly required for the requested change; do not widen the change set into unrelated user-owned or parallel worktree edits, do not touch changes owned by another agent, and if a blocker comes from that parallel work, wait briefly and re-check instead of patching around their in-flight fix.
@@ -140,7 +141,7 @@ Rule format:
140141
### Commands
141142

142143
- `build`: `dotnet build ./PrompterOne.slnx -warnaserror`
143-
- `test`: `dotnet test @./tests/dotnet-test-progress.rsp --solution ./PrompterOne.slnx --max-parallel-test-modules 1`
144+
- `test`: `dotnet test @./tests/dotnet-test-progress.rsp --solution ./PrompterOne.slnx`
144145
- `format`: `dotnet format ./PrompterOne.slnx`
145146
- `coverage`: `dotnet test @./tests/dotnet-test-progress.rsp --project ./tests/PrompterOne.Core.Tests/PrompterOne.Core.Tests.csproj --coverage --coverage-output-format cobertura && dotnet test @./tests/dotnet-test-progress.rsp --project ./tests/PrompterOne.Web.Tests/PrompterOne.Web.Tests.csproj --coverage --coverage-output-format cobertura && dotnet test @./tests/dotnet-test-progress.rsp --project ./tests/PrompterOne.Web.UITests.Shell/PrompterOne.Web.UITests.Shell.csproj --coverage --coverage-output-format cobertura && dotnet test @./tests/dotnet-test-progress.rsp --project ./tests/PrompterOne.Web.UITests.Studio/PrompterOne.Web.UITests.Studio.csproj --coverage --coverage-output-format cobertura && dotnet test @./tests/dotnet-test-progress.rsp --project ./tests/PrompterOne.Web.UITests.Editor/PrompterOne.Web.UITests.Editor.csproj --coverage --coverage-output-format cobertura && dotnet test @./tests/dotnet-test-progress.rsp --project ./tests/PrompterOne.Web.UITests.Reader/PrompterOne.Web.UITests.Reader.csproj --coverage --coverage-output-format cobertura`
146147

@@ -158,7 +159,7 @@ Useful focused commands:
158159
- app run: `cd ./src/PrompterOne.Web && dotnet run`
159160
- core tests: `dotnet test @./tests/dotnet-test-progress.rsp --project ./tests/PrompterOne.Core.Tests/PrompterOne.Core.Tests.csproj`
160161
- component tests: `dotnet test @./tests/dotnet-test-progress.rsp --project ./tests/PrompterOne.Web.Tests/PrompterOne.Web.Tests.csproj`
161-
- all tests: `dotnet test @./tests/dotnet-test-progress.rsp --solution ./PrompterOne.slnx --max-parallel-test-modules 1`
162+
- all tests: `dotnet test @./tests/dotnet-test-progress.rsp --solution ./PrompterOne.slnx`
162163
- ui tests: `dotnet test @./tests/dotnet-test-progress.rsp --project ./tests/PrompterOne.Web.UITests.Shell/PrompterOne.Web.UITests.Shell.csproj && dotnet test @./tests/dotnet-test-progress.rsp --project ./tests/PrompterOne.Web.UITests.Studio/PrompterOne.Web.UITests.Studio.csproj && dotnet test @./tests/dotnet-test-progress.rsp --project ./tests/PrompterOne.Web.UITests.Editor/PrompterOne.Web.UITests.Editor.csproj && dotnet test @./tests/dotnet-test-progress.rsp --project ./tests/PrompterOne.Web.UITests.Reader/PrompterOne.Web.UITests.Reader.csproj`
163164
- ui shell tests: `dotnet test @./tests/dotnet-test-progress.rsp --project ./tests/PrompterOne.Web.UITests.Shell/PrompterOne.Web.UITests.Shell.csproj`
164165
- ui studio tests: `dotnet test @./tests/dotnet-test-progress.rsp --project ./tests/PrompterOne.Web.UITests.Studio/PrompterOne.Web.UITests.Studio.csproj`

full-suite-stabilization.plan.md

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,12 +96,20 @@ Tracked failing tests from the current baseline:
9696
Fix path:
9797
- remove the CI-only per-context routed warmup, keep only the one-time shared-runtime warmup, and make every returned test page follow the same bootstrap path locally and on CI
9898
- [ ] `StandaloneAppFixture` build break: `WarmUpContextPageIfNeededAsync` missing in `StandaloneAppFixture.cs`
99+
- [x] `StandaloneAppFixture` build break: `WarmUpContextPageIfNeededAsync` missing in `StandaloneAppFixture.cs`
99100
Symptom:
100101
- compiler reports `CS0103` for `WarmUpContextPageIfNeededAsync` from `tests/PrompterOne.Web.UITests/Infrastructure/StandaloneAppFixture.cs`
101102
Root cause:
102-
- verify whether the current workspace already contains the moved helper in `StandaloneAppFixture.Warmup.cs` and whether the failing build came from an older not-yet-published tree versus a real current partial-class mismatch
103+
- the warmup helper had already been split into the partial fixture, but the published tree and the active fixture callers drifted until the shared warmup path and callers were brought back into the same compiled partial surface
103104
Fix path:
104-
- confirm the helper is present in the compiled partial, keep the calling signature aligned, and re-run the required local build after the active full-suite run completes
105+
- keep the helper in the compiled partial, align the callers with that partial surface, and prove the fix with the required local build and full solution test commands
106+
- [x] `EditorDragDropFlowTests.EditorScreen_DropOnEmptyDraft_ReplacesTextAndSupportsUndoRedo`
107+
Symptom:
108+
- the blank-draft drop scenario updated the source text and title but intermittently left toolbar undo disabled, especially once untitled autosave assigned a real `?id=` route
109+
Root cause:
110+
- dropping onto an untitled draft triggered autosave self-navigation that reloaded the editor and reset document history, so slower runs could lose undo state before the assertion clicked it
111+
Fix path:
112+
- preserve editor history across untitled autosave self-navigation and make the browser regression wait for the persisted route before asserting undo/redo on the post-save editor surface
105113
- [ ] `Release Pipeline` run `24209085607` still fails on GitHub macOS despite the local full-suite baseline being green on the same `727d904` commit
106114
Symptom:
107115
- `Shell` and `Studio` fail remotely while local `dotnet build`, targeted suite runs, and the required solution-level `dotnet test` all pass on the exact pushed commit
@@ -197,6 +205,24 @@ Tracked failing tests from the current baseline:
197205
- detect when `BrowserRouteDriver` is opening the first routed page from the primed blank test page
198206
- give only that first routed bootstrap the longer runtime-warmup visibility budget and skip the CI blank bounce for that specific path
199207
- keep the shorter route-visible contract for already-booted routed pages so normal suite latency does not drift upward
208+
- [ ] `Release Pipeline` run `24221639706` fails remotely in all four browser suites after commit `7983efe`
209+
Symptom:
210+
- `Shell` fails `11` tests, mostly while opening `/library`, plus two invalid learn/teleprompter missing-script flows and two onboarding route-changing clicks
211+
- `Studio` fails `6` tests, split between first-route `/library` boot, `go-live-back` hangs, and `StartRecording` click paths that stall on scheduled navigation waits
212+
- `Reader` fails `3` tests: two responsive-layout screenshot captures on iPad Pro portrait and one teleprompter muted-chrome visual threshold
213+
- `Editor` fails multiple route-changing action tests, including `Open in Library` from split results and import/drag-drop flows that do not wait on the real completion signal
214+
Root cause:
215+
- `BrowserRouteDriver` still lets `TimeoutException` escape from the page-visible probe, so the shared route retry loop is bypassed on CI
216+
- the harness still contains route/open divergence between runtime warmup and scenario route opens, and shared contexts can be published before priming fully succeeds
217+
- several suites still use raw SPA clicks that wait for scheduled navigation even though the tests already do explicit post-click route or readiness waits
218+
- responsive screenshot capture is still a one-shot operation, so artifact generation itself can fail otherwise healthy route assertions
219+
- a production Go Live route-leave path still blocks navigation on camera detach work
220+
Fix path:
221+
- make route-open retries catch timeout failures at the shared driver boundary and remove CI-only route-open behavior
222+
- publish shared contexts only after priming succeeds and reuse the shared route driver from runtime warmup
223+
- introduce a shared `NoWaitAfter` click helper for SPA route/state-changing controls and use it in the failing Shell, Studio, and Editor flows
224+
- retry page screenshot capture in the shared artifact helper
225+
- stop awaiting camera detach inside `GoLivePage` location-changing so route leaves are not blocked by cleanup
200226

201227
## Ordered Plan
202228

@@ -234,6 +260,7 @@ Tracked failing tests from the current baseline:
234260
- `dotnet format ./PrompterOne.slnx` passed
235261
- `dotnet build ./PrompterOne.slnx -warnaserror` passed
236262
- `dotnet test @./tests/dotnet-test-progress.rsp --solution ./PrompterOne.slnx --max-parallel-test-modules 1` passed with `1162/1162` green in `7m 42.943s`
263+
- post-format verification repeated successfully with `dotnet build ./PrompterOne.slnx -warnaserror` and `dotnet test @./tests/dotnet-test-progress.rsp --solution ./PrompterOne.slnx --max-parallel-test-modules 1`, ending at `1162/1162` green in `7m 35.996s`
237264

238265
- [ ] Step 5. Publish directly to `main`.
239266
Actions:
@@ -258,6 +285,12 @@ Tracked failing tests from the current baseline:
258285
- `dotnet format ./PrompterOne.slnx` passed
259286
- `dotnet build ./PrompterOne.slnx -warnaserror` passed
260287
- `dotnet test @./tests/dotnet-test-progress.rsp --project ./tests/PrompterOne.Web.UITests.Studio/PrompterOne.Web.UITests.Studio.csproj` passed with `38/38`
288+
- [ ] Follow-up remediation for remote run `24221639706`
289+
Pending focused validation:
290+
- `dotnet test @./tests/dotnet-test-progress.rsp --project ./tests/PrompterOne.Web.UITests.Shell/PrompterOne.Web.UITests.Shell.csproj`
291+
- `dotnet test @./tests/dotnet-test-progress.rsp --project ./tests/PrompterOne.Web.UITests.Studio/PrompterOne.Web.UITests.Studio.csproj`
292+
- `dotnet test @./tests/dotnet-test-progress.rsp --project ./tests/PrompterOne.Web.UITests.Reader/PrompterOne.Web.UITests.Reader.csproj`
293+
- `dotnet test @./tests/dotnet-test-progress.rsp --project ./tests/PrompterOne.Web.UITests.Editor/PrompterOne.Web.UITests.Editor.csproj`
261294
- `dotnet test @./tests/dotnet-test-progress.rsp --solution ./PrompterOne.slnx --max-parallel-test-modules 1` passed with `1162/1162` green in `7m 50.591s`
262295
- [x] Follow-up local verification after the first-route primed-blank bootstrap change
263296
Result:

src/PrompterOne.Shared/Editor/Pages/EditorPage.Loading.cs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,10 @@ await Diagnostics.RunAsync(
3030
{
3131
await Bootstrapper.EnsureReadyAsync();
3232
await EnsureSessionLoadedAsync();
33+
var preserveHistoryOnLoad = ConsumePreserveHistoryOnNextLoad();
3334
await LoadEditorFileWorkflowAsync();
3435
PopulateEditorState(
35-
resetHistory: true,
36+
resetHistory: !preserveHistoryOnLoad,
3637
clearSplitFeedback: !ConsumePreserveSplitFeedbackOnNextLoad());
3738
StateHasChanged();
3839
});
@@ -57,18 +58,21 @@ private void UpdateDraftSessionOrigin(string? requestedScriptId)
5758
{
5859
_currentDraftSessionStartedUntitled = true;
5960
_pendingAutosaveSelfNavigationScriptId = null;
61+
_preserveHistoryOnNextLoad = false;
6062
return;
6163
}
6264

6365
if (_currentDraftSessionStartedUntitled &&
6466
string.Equals(requestedScriptId, _pendingAutosaveSelfNavigationScriptId, StringComparison.Ordinal))
6567
{
6668
_pendingAutosaveSelfNavigationScriptId = null;
69+
_preserveHistoryOnNextLoad = true;
6770
return;
6871
}
6972

7073
_currentDraftSessionStartedUntitled = false;
7174
_pendingAutosaveSelfNavigationScriptId = null;
75+
_preserveHistoryOnNextLoad = false;
7276
}
7377

7478
private async Task LoadScriptFromQueryAsync(string requestedScriptId)
@@ -129,6 +133,13 @@ private bool ConsumePreserveSplitFeedbackOnNextLoad()
129133
return shouldPreserveSplitFeedback;
130134
}
131135

136+
private bool ConsumePreserveHistoryOnNextLoad()
137+
{
138+
var shouldPreserveHistory = _preserveHistoryOnNextLoad;
139+
_preserveHistoryOnNextLoad = false;
140+
return shouldPreserveHistory;
141+
}
142+
132143
private void ResetMetadataDefaults(ScriptWorkspaceState state)
133144
{
134145
var defaultBaseWpm = state.ScriptData?.TargetWpm ?? 140;

src/PrompterOne.Shared/Editor/Pages/EditorPage.SourceEditing.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ private async Task ApplyMutationAsync(string text, EditorSelectionRange selectio
121121
PersistDraftInBackground(text, documentNameOverride);
122122
}
123123

124+
await InvokeAsync(StateHasChanged);
124125
await FocusSourceRangeAsync(selection.Start, selection.End, revealSelection: false);
125126
}
126127

src/PrompterOne.Shared/Editor/Pages/EditorPage.razor.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ public partial class EditorPage
5353
private DateTimeOffset? _lastLocalSaveAt;
5454
private EditorMetadataRailTab _metadataRailSelectedTab = EditorMetadataRailTab.Metadata;
5555
private string _profile = DefaultProfileActor;
56+
private bool _preserveHistoryOnNextLoad;
5657
private bool _preserveSplitFeedbackOnNextLoad;
5758
private string? _pendingAutosaveSelfNavigationScriptId;
5859
private string _screenTitle = ScriptWorkspaceState.UntitledScriptTitle;

src/PrompterOne.Shared/GoLive/Pages/GoLivePage.Notifications.cs

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
using Microsoft.AspNetCore.Components.Routing;
2+
using Microsoft.JSInterop;
23
using PrompterOne.Shared.Contracts;
4+
using PrompterOne.Shared.GoLive.Models;
35

46
namespace PrompterOne.Shared.Pages;
57

68
public partial class GoLivePage
79
{
10+
private const string BackgroundMaintenanceOperation = "GoLiveBackgroundMaintenance";
811
private static readonly string GoLiveRoutePrefix = AppRoutes.GoLive.TrimStart('/');
912
private static readonly TimeSpan SessionRefreshInterval = TimeSpan.FromMilliseconds(200);
1013

@@ -30,9 +33,9 @@ public void Dispose()
3033
}
3134

3235
_disposed = true;
33-
_ = ReleaseCameraSurfacesAsync();
34-
_ = StopPrimaryMicrophoneMonitorAsync();
35-
_ = GoLiveRemoteSourceRuntime.StopAsync();
36+
RunCleanupInBackground(ReleaseCameraSurfacesAsync);
37+
RunCleanupInBackground(StopPrimaryMicrophoneMonitorAsync);
38+
RunCleanupInBackground(GoLiveRemoteSourceRuntime.StopAsync);
3639
DisposeCore();
3740
GC.SuppressFinalize(this);
3841
}
@@ -45,9 +48,9 @@ public async ValueTask DisposeAsync()
4548
}
4649

4750
_disposed = true;
48-
await ReleaseCameraSurfacesAsync();
49-
await StopPrimaryMicrophoneMonitorAsync();
50-
await GoLiveRemoteSourceRuntime.StopAsync();
51+
await RunCleanupSafelyAsync(ReleaseCameraSurfacesAsync);
52+
await RunCleanupSafelyAsync(StopPrimaryMicrophoneMonitorAsync);
53+
await RunCleanupSafelyAsync(GoLiveRemoteSourceRuntime.StopAsync);
5154
DisposeCore();
5255
GC.SuppressFinalize(this);
5356
}
@@ -114,6 +117,13 @@ private async Task RunSessionRefreshLoopAsync(PeriodicTimer timer, CancellationT
114117
catch (OperationCanceledException)
115118
{
116119
}
120+
catch (Exception exception) when (IsIgnorableBackgroundFailure(exception))
121+
{
122+
}
123+
catch (Exception exception)
124+
{
125+
ReportBackgroundFailure(exception);
126+
}
117127
}
118128

119129
private void StopSessionRefreshLoop()
@@ -132,7 +142,10 @@ private ValueTask HandleLocationChangingAsync(LocationChangingContext context)
132142
return ValueTask.CompletedTask;
133143
}
134144

135-
return new ValueTask(ReleaseCameraSurfacesAsync());
145+
StopSessionRefreshLoop();
146+
RunCleanupInBackground(ReleaseCameraSurfacesAsync);
147+
RunCleanupInBackground(StopPrimaryMicrophoneMonitorAsync);
148+
return ValueTask.CompletedTask;
136149
}
137150

138151
private bool IsGoLiveTarget(string targetLocation)
@@ -152,4 +165,42 @@ private void DisposeCore()
152165
DisposePrimaryMicrophoneObserver();
153166
_interactionGate.Dispose();
154167
}
168+
169+
private void RunCleanupInBackground(Func<Task> cleanup) =>
170+
_ = RunCleanupSafelyAsync(cleanup);
171+
172+
private async Task RunCleanupSafelyAsync(Func<Task> cleanup)
173+
{
174+
try
175+
{
176+
await cleanup();
177+
}
178+
catch (Exception exception) when (IsIgnorableBackgroundFailure(exception))
179+
{
180+
}
181+
catch (Exception exception)
182+
{
183+
ReportBackgroundFailure(exception);
184+
}
185+
}
186+
187+
private static bool IsIgnorableBackgroundFailure(Exception exception) =>
188+
exception is OperationCanceledException
189+
or ObjectDisposedException
190+
or TaskCanceledException
191+
or JSException;
192+
193+
private void ReportBackgroundFailure(Exception exception)
194+
{
195+
if (_disposed)
196+
{
197+
return;
198+
}
199+
200+
Diagnostics.ReportRecoverable(
201+
BackgroundMaintenanceOperation,
202+
Text(GoLiveText.Load.LoadMessage),
203+
exception,
204+
alreadyLogged: false);
205+
}
155206
}

0 commit comments

Comments
 (0)