From f629b78320261b11786d56a4dad4605d813ae3a7 Mon Sep 17 00:00:00 2001 From: Ivan Vydrin Date: Thu, 30 Jul 2026 21:50:45 +0300 Subject: [PATCH 1/5] Stop the browser suite from racing the app it is driving Three separate failures during today's releases were one bug wearing different hats: an assertion that reads the DOM once, against a dashboard that re-renders asynchronously. `Assert.Equal(14, await rows.CountAsync())` takes whatever is on screen at that instant, so on a runner already busy with eleven hundred unit tests across two frameworks it caught the board half-built and reported a broken feature. Playwright's own assertions retry until the page settles or the timeout expires, which is what these always wanted. Twenty-four such reads become `Assertions.Expect(...)`. The readiness signal moves with them: waiting for the first row only proves the list has started, so `WaitForTheBoardAsync` waits for every checker before a test may read anything, which is what makes the counts the remaining tests compare safe at all. Three fixed sleeps go: two guessed at a theme switch, and the third guessed at a write to the state provider, where the event-log entry that write produces is the real signal. Failures are diagnosable now too. Each test gets its own browser context with tracing on, and a test that fails keeps its trace; CI uploads them from both the pull-request and the release workflow. A browser failure that happens only on a runner was a stack trace and a guess -- three guesses, as it turned out. It is now a zip with the DOM, the network and a screenshot at the moment it gave up. --- .github/workflows/ci.yml | 13 ++++ .github/workflows/publish.yml | 13 ++++ tests/Healthie.Tests.E2E/BrowserFixture.cs | 67 ++++++++++++++++- .../DashboardGroupingTests.cs | 39 +++++----- .../DashboardReadOnlyTests.cs | 21 +++--- tests/Healthie.Tests.E2E/DashboardTests.cs | 72 ++++++++++++------- 6 files changed, 172 insertions(+), 53 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index de411c8..8d544fd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -71,3 +71,16 @@ jobs: run: > dotnet test tests/Healthie.Tests.E2E --configuration Release --no-build --verbosity normal + + # A browser test that fails only on a runner is otherwise a stack trace and a guess. The suite + # records a Playwright trace per test and keeps the ones that failed; this is what makes them + # readable afterwards -- open a downloaded zip at https://trace.playwright.dev and you get the + # DOM, the network and a screenshot at the moment it gave up. + - name: Upload failure traces + if: failure() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: playwright-traces + path: tests/Healthie.Tests.E2E/bin/Release/net8.0/playwright-traces + if-no-files-found: ignore + retention-days: 7 diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 16d852a..a1e9ddd 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -122,6 +122,19 @@ jobs: --no-build --verbosity normal + # This is where a browser failure costs the most: the release stops, and all anybody has is a + # stack trace from a runner nobody can reach. The suite keeps a Playwright trace for each test + # that failed -- open the downloaded zip at https://trace.playwright.dev for the DOM, the + # network and a screenshot at the moment it gave up. + - name: Upload failure traces + if: failure() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: playwright-traces + path: tests/Healthie.Tests.E2E/bin/${{ env.CONFIGURATION }}/net8.0/playwright-traces + if-no-files-found: ignore + retention-days: 7 + - name: Pack run: > dotnet pack diff --git a/tests/Healthie.Tests.E2E/BrowserFixture.cs b/tests/Healthie.Tests.E2E/BrowserFixture.cs index 617f676..48a8047 100644 --- a/tests/Healthie.Tests.E2E/BrowserFixture.cs +++ b/tests/Healthie.Tests.E2E/BrowserFixture.cs @@ -1,4 +1,4 @@ -using System.Collections.Concurrent; +using System.Collections.Concurrent; using Microsoft.Playwright; namespace Healthie.Tests.E2E; @@ -10,6 +10,8 @@ namespace Healthie.Tests.E2E; public sealed class BrowserFixture : IAsyncLifetime { private readonly ConcurrentDictionary> _errors = new(); + private readonly ConcurrentDictionary _contexts = new(); + private readonly ConcurrentDictionary _owners = new(); private IPlaywright? _playwright; private IBrowser? _browser; @@ -41,9 +43,70 @@ public async ValueTask InitializeAsync() public async Task NewPageAsync() { var browser = _browser ?? throw new InvalidOperationException("The browser is not running."); - return await browser.NewPageAsync(new() { ViewportSize = new() { Width = 1440, Height = 900 } }); + + var context = await browser.NewContextAsync(new() { ViewportSize = new() { Width = 1440, Height = 900 } }); + + // Recorded for every test and kept only for the ones that fail. A browser test that fails on + // a runner and passes everywhere else is otherwise a stack trace and a guess: this leaves the + // DOM, the network and a screenshot at the moment it gave up, which is the difference between + // diagnosing it once and patching it three times. + await context.Tracing.StartAsync(new() { Screenshots = true, Snapshots = true, Sources = false }); + + var page = await context.NewPageAsync(); + _contexts[page] = context; + + // Keyed by the running test so a class that opens several pages closes only its own. + if (TestContext.Current.Test is { } test) + { + _owners[page] = test; + } + + return page; + } + + /// + /// Closes every page the current test opened, keeping a trace only if it failed. + /// + /// + /// Kept only on failure because a trace is a few megabytes and fifty-three passing ones are worth + /// nothing. is what CI uploads. + /// + public async Task FinishCurrentTestAsync() + { + var test = TestContext.Current.Test; + var failed = TestContext.Current.TestState?.Result == TestResult.Failed; + var name = test?.TestDisplayName ?? "unknown-test"; + + foreach (var (page, context) in _contexts.ToArray()) + { + if (!ReferenceEquals(_owners.GetValueOrDefault(page), test)) + { + continue; + } + + _contexts.TryRemove(page, out _); + _owners.TryRemove(page, out _); + + if (failed) + { + Directory.CreateDirectory(TraceDirectory); + + var safe = string.Concat(name.Select(c => char.IsLetterOrDigit(c) || c is '-' or '_' ? c : '-')); + await context.Tracing.StopAsync(new() { Path = Path.Combine(TraceDirectory, $"{safe}.zip") }); + } + else + { + await context.Tracing.StopAsync(); + } + + await context.CloseAsync(); + } } + /// Where failure traces are written, and what CI collects. + public static string TraceDirectory { get; } = + Path.Combine(AppContext.BaseDirectory, "playwright-traces"); + /// Records the console and page errors seen on a page, for . public void TrackErrors(IPage page, List errors) => _errors[page] = errors; diff --git a/tests/Healthie.Tests.E2E/DashboardGroupingTests.cs b/tests/Healthie.Tests.E2E/DashboardGroupingTests.cs index ec9a625..4b0dd03 100644 --- a/tests/Healthie.Tests.E2E/DashboardGroupingTests.cs +++ b/tests/Healthie.Tests.E2E/DashboardGroupingTests.cs @@ -14,8 +14,11 @@ namespace Healthie.Tests.E2E; /// is the test that would have caught it. /// [Collection(nameof(BrowserCollection))] -public class DashboardGroupingTests(BrowserFixture browser) +public class DashboardGroupingTests(BrowserFixture browser) : IAsyncDisposable { + /// Closes the pages this test opened, keeping a trace behind if it failed. + public async ValueTask DisposeAsync() => await browser.FinishCurrentTestAsync(); + private static CancellationToken Ct => TestContext.Current.CancellationToken; /// Grouped and tagged by the sample, so it exercises both concepts at once. @@ -60,7 +63,7 @@ private async Task OpenDashboardAsync(SampleApp app) browser.TrackErrors(page, errors); await page.GotoAsync(app.DashboardUrl, new() { WaitUntil = WaitUntilState.NetworkIdle }); - await page.Locator(".hpm-row").First.WaitForAsync(new() { Timeout = 30_000 }); + await DashboardTests.WaitForTheBoardAsync(page); return page; } @@ -94,18 +97,16 @@ public async Task Dashboard_WhenGrouped_ShowsEveryCheckerExactlyOnce(ProviderSet var page = await OpenDashboardAsync(app); // The board opens grouped, so this is the state under test without touching anything. - await page.Locator(".hpm-group").First.WaitForAsync(); - - var groupedCount = await page.Locator(".hpm-row").CountAsync(); - Assert.Equal(1, await RowFor(page, TargetChecker).CountAsync()); + await Assertions.Expect(page.Locator(".hpm-group").First).ToBeVisibleAsync(); + await Assertions.Expect(RowFor(page, TargetChecker)).ToHaveCountAsync(1); // The same checkers laid out flat: the button goes the other way now, which makes this the // comparison the test always wanted rather than a count taken before grouping was applied. await page.GetByRole(AriaRole.Button, new() { Name = "GROUP", Exact = true }).ClickAsync(); await Assertions.Expect(page.Locator(".hpm-group")).ToHaveCountAsync(0); - Assert.Equal(groupedCount, await page.Locator(".hpm-row").CountAsync()); - Assert.Equal(1, await RowFor(page, TargetChecker).CountAsync()); + await Assertions.Expect(page.Locator(".hpm-row")).ToHaveCountAsync(DashboardTests.CheckerCount); + await Assertions.Expect(RowFor(page, TargetChecker)).ToHaveCountAsync(1); browser.AssertNoErrors(page); } @@ -282,11 +283,16 @@ public async Task Dashboard_FilteringByATag_ShowsOnlyTheCheckersCarryingIt(Provi await page.Locator(".hpm-tag-filter").SelectOptionAsync("tier-1"); await Assertions.Expect(page.Locator(".hpm-row")).Not.ToHaveCountAsync(total); - var shown = await page.Locator(".hpm-row").CountAsync(); - Assert.True(shown > 0, "filtering by a tag the sample uses should leave something on screen"); + Assert.True( + await page.Locator(".hpm-row").CountAsync() > 0, + "filtering by a tag the sample uses should leave something on screen"); - // Every row left has to carry the tag, which is the only claim the filter makes. - Assert.Equal(shown, await page.Locator(".hpm-row").Filter(new() { Has = page.Locator(".hpm-chip", new() { HasTextString = "tier-1" }) }).CountAsync()); + // Every row left has to carry the tag, which is the only claim the filter makes. Stated as + // "nothing without it", so it is one assertion over the live list rather than two counts + // read a moment apart and compared. + await Assertions.Expect(page.Locator(".hpm-row") + .Filter(new() { HasNot = page.Locator(".hpm-chip", new() { HasTextString = "tier-1" }) })) + .ToHaveCountAsync(0); browser.AssertNoErrors(page); } @@ -306,7 +312,8 @@ public async Task Dashboard_PinningAChecker_SortsItFirstAndSurvivesAReload(Provi Assert.Equal(TargetChecker, await FirstRowNameAsync(page)); await page.ReloadAsync(new() { WaitUntil = WaitUntilState.NetworkIdle }); - await page.Locator(".hpm-row").First.WaitForAsync(); + await DashboardTests.WaitForTheBoardAsync(page); + await Assertions.Expect(page.Locator(".hpm-row").First.Locator(".hpm-pin-mark")).ToBeVisibleAsync(); Assert.Equal(TargetChecker, await FirstRowNameAsync(page)); browser.AssertNoErrors(page); @@ -329,7 +336,7 @@ public async Task Dashboard_AddingATag_ShowsItOnTheRowAndSurvivesAReload(Provide await Assertions.Expect(tagged).ToBeVisibleAsync(); await page.ReloadAsync(new() { WaitUntil = WaitUntilState.NetworkIdle }); - await page.Locator(".hpm-row").First.WaitForAsync(); + await DashboardTests.WaitForTheBoardAsync(page); await Assertions.Expect(page.Locator(".hpm-row").Filter(new() { HasTextString = TargetChecker }) .Locator(".hpm-chip", new() { HasTextString = "e2e-added" })).ToBeVisibleAsync(); @@ -343,12 +350,10 @@ public async Task Dashboard_SwitchingToCards_KeepsEveryCheckerOnScreen(ProviderS await using var app = await SampleApp.StartAsync(setup, Ct); var page = await OpenDashboardAsync(app); - var total = await page.Locator(".hpm-row").CountAsync(); - await page.GetByRole(AriaRole.Button, new() { Name = "Card view" }).ClickAsync(); await Assertions.Expect(page.Locator(".hpm-list--cards")).ToBeVisibleAsync(); - Assert.Equal(total, await page.Locator(".hpm-row").CountAsync()); + await Assertions.Expect(page.Locator(".hpm-row")).ToHaveCountAsync(DashboardTests.CheckerCount); browser.AssertNoErrors(page); } diff --git a/tests/Healthie.Tests.E2E/DashboardReadOnlyTests.cs b/tests/Healthie.Tests.E2E/DashboardReadOnlyTests.cs index 6264885..37fbdf3 100644 --- a/tests/Healthie.Tests.E2E/DashboardReadOnlyTests.cs +++ b/tests/Healthie.Tests.E2E/DashboardReadOnlyTests.cs @@ -17,8 +17,11 @@ namespace Healthie.Tests.E2E; /// /// [Collection(nameof(BrowserCollection))] -public class DashboardReadOnlyTests(BrowserFixture browser) +public class DashboardReadOnlyTests(BrowserFixture browser) : IAsyncDisposable { + /// Closes the pages this test opened, keeping a trace behind if it failed. + public async ValueTask DisposeAsync() => await browser.FinishCurrentTestAsync(); + private static CancellationToken Ct => TestContext.Current.CancellationToken; private static readonly ProviderSetup Setup = new("Timer", UseCosmos: false); @@ -46,7 +49,7 @@ private async Task OpenDashboardAsync(SampleApp app) browser.TrackErrors(page, errors); await page.GotoAsync(app.DashboardUrl, new() { WaitUntil = WaitUntilState.NetworkIdle }); - await page.Locator(".hpm-row").First.WaitForAsync(new() { Timeout = 30_000 }); + await DashboardTests.WaitForTheBoardAsync(page); return page; } @@ -99,15 +102,15 @@ public async Task Dashboard_WithMutationsOff_StillReportsEverything() // A check has to land before there is any history to draw. await page.Locator(".hpm-blips").First.WaitForAsync(new() { Timeout = 30_000 }); - Assert.True(await page.Locator(".hpm-row").CountAsync() > 0, "no checkers are listed"); - Assert.True(await page.Locator(".hpm-stats").CountAsync() > 0, "the selected checker's stats are missing"); - Assert.Equal(1, await page.Locator(".hpm-log-body").CountAsync()); + await Assertions.Expect(page.Locator(".hpm-row").First).ToBeVisibleAsync(); + await Assertions.Expect(page.Locator(".hpm-stats").First).ToBeVisibleAsync(); + await Assertions.Expect(page.Locator(".hpm-log-body")).ToHaveCountAsync(1); // The controls that only change what the viewer sees are not mutations, so they stay. - Assert.Equal(1, await page.Locator("[aria-label='Search checkers']").CountAsync()); - Assert.Equal(1, await page.Locator("button:text-is('GROUP')").CountAsync()); - Assert.Equal(1, await page.Locator("[aria-label='Legend and about']").CountAsync()); - Assert.Equal(1, await page.Locator("[aria-label='Expand the event log']").CountAsync()); + await Assertions.Expect(page.Locator("[aria-label='Search checkers']")).ToHaveCountAsync(1); + await Assertions.Expect(page.Locator("button:text-is('GROUP')")).ToHaveCountAsync(1); + await Assertions.Expect(page.Locator("[aria-label='Legend and about']")).ToHaveCountAsync(1); + await Assertions.Expect(page.Locator("[aria-label='Expand the event log']")).ToHaveCountAsync(1); browser.AssertNoErrors(page); } diff --git a/tests/Healthie.Tests.E2E/DashboardTests.cs b/tests/Healthie.Tests.E2E/DashboardTests.cs index d9ba99a..8863385 100644 --- a/tests/Healthie.Tests.E2E/DashboardTests.cs +++ b/tests/Healthie.Tests.E2E/DashboardTests.cs @@ -8,8 +8,11 @@ namespace Healthie.Tests.E2E; /// the Blazor circuit, the CSS, or the render loop fails the test -- none of which a unit test sees. /// [Collection(nameof(BrowserCollection))] -public class DashboardTests(BrowserFixture browser) +public class DashboardTests(BrowserFixture browser) : IAsyncDisposable { + /// Closes the pages this test opened, keeping a trace behind if it failed. + public async ValueTask DisposeAsync() => await browser.FinishCurrentTestAsync(); + private static CancellationToken Ct => TestContext.Current.CancellationToken; /// @@ -43,6 +46,13 @@ public static TheoryData Setups /// private const string TargetChecker = "Redis Cache"; + /// How many pulse checkers the sample declares. + /// + /// Asserted rather than ignored: the board losing a row is exactly the kind of regression a + /// "some rows are present" check waves through. + /// + internal const int CheckerCount = 14; + private static ILocator RowFor(IPage page, string displayName) => page.Locator(".hpm-row").Filter(new() { HasTextString = displayName }); @@ -55,11 +65,23 @@ private async Task OpenDashboardAsync(SampleApp app) browser.TrackErrors(page, errors); await page.GotoAsync(app.DashboardUrl, new() { WaitUntil = WaitUntilState.NetworkIdle }); - // Rows only appear once the checkers have been read, so this is the real readiness signal. - await page.Locator(".hpm-row").First.WaitForAsync(new() { Timeout = 30_000 }); + await WaitForTheBoardAsync(page); return page; } + /// + /// Waits until the board is showing every checker, which is what "loaded" means here. + /// + /// + /// Rows arrive as the checkers are read, so waiting for the first one only proves the list has + /// started. A count taken at that moment can catch it half-built -- and a count is the thing + /// most of these tests then compare against, which is how a suite that passes on a developer's + /// machine fails on a loaded runner. Waiting for the full set here is what makes reading a + /// count downstream safe at all. + /// + internal static Task WaitForTheBoardAsync(IPage page) => + Assertions.Expect(page.Locator(".hpm-row")).ToHaveCountAsync(CheckerCount, new() { Timeout = 30_000 }); + [Theory] [MemberData(nameof(Setups))] public async Task Dashboard_ListsEveryChecker_AndReportsNoBrowserErrors(ProviderSetup setup) @@ -67,8 +89,8 @@ public async Task Dashboard_ListsEveryChecker_AndReportsNoBrowserErrors(Provider await using var app = await SampleApp.StartAsync(setup, Ct); var page = await OpenDashboardAsync(app); - Assert.Equal(14, await page.Locator(".hpm-row").CountAsync()); - Assert.Equal("HEALTHIE·PULSE", (await page.Locator(".hpm-wordmark").TextContentAsync())?.Trim()); + await Assertions.Expect(page.Locator(".hpm-row")).ToHaveCountAsync(CheckerCount); + await Assertions.Expect(page.Locator(".hpm-wordmark")).ToHaveTextAsync("HEALTHIE·PULSE"); browser.AssertNoErrors(page); } @@ -98,9 +120,7 @@ public async Task SelectingAChecker_ShowsItsDetail(ProviderSetup setup) await RowFor(page, TargetChecker).ClickAsync(); - await page.Locator(".hpm-sel-name", new() { HasTextString = TargetChecker }) - .WaitForAsync(new() { Timeout = 10_000 }); - Assert.Equal(TargetChecker, (await page.Locator(".hpm-sel-name").TextContentAsync())?.Trim()); + await Assertions.Expect(page.Locator(".hpm-sel-name")).ToHaveTextAsync(TargetChecker); // A count would be a timing assertion, not a detail one: the sample installs the uptime // package, whose 24H cell appears once a segment has been recorded and whose WORST cell @@ -125,12 +145,11 @@ public async Task Search_FiltersTheList_AndRestoresIt(ProviderSetup setup) var page = await OpenDashboardAsync(app); await page.FillAsync(".hpm-search input", "no-such-checker"); - await page.Locator(".hpm-empty").WaitForAsync(new() { Timeout = 10_000 }); - Assert.Equal(0, await page.Locator(".hpm-row").CountAsync()); + await Assertions.Expect(page.Locator(".hpm-row")).ToHaveCountAsync(0); + await Assertions.Expect(page.Locator(".hpm-empty")).ToBeVisibleAsync(); await page.FillAsync(".hpm-search input", ""); - await page.Locator(".hpm-row").First.WaitForAsync(new() { Timeout = 10_000 }); - Assert.Equal(14, await page.Locator(".hpm-row").CountAsync()); + await Assertions.Expect(page.Locator(".hpm-row")).ToHaveCountAsync(CheckerCount); browser.AssertNoErrors(page); } @@ -145,8 +164,7 @@ public async Task RunAll_ProducesEventLogEntries(ProviderSetup setup) await page.Locator(".hpm-btn", new() { HasTextString = "RUN ALL" }).ClickAsync(); - await page.Locator(".hpm-event").First.WaitForAsync(new() { Timeout = 20_000 }); - Assert.True(await page.Locator(".hpm-event").CountAsync() > 0); + await Assertions.Expect(page.Locator(".hpm-event").First).ToBeVisibleAsync(); browser.AssertNoErrors(page); } @@ -160,19 +178,23 @@ public async Task ChangingTheInterval_PersistsThroughTheStateProvider(ProviderSe var page = await OpenDashboardAsync(app); await RowFor(page, TargetChecker).ClickAsync(); - await page.Locator("#hpm-interval").WaitForAsync(new() { Timeout = 10_000 }); + await Assertions.Expect(page.Locator(".hpm-sel-name")).ToHaveTextAsync(TargetChecker); await page.SelectOptionAsync("#hpm-interval", "Every5Minutes"); - await page.WaitForTimeoutAsync(1500); + + // The event log entry is written once the write to the provider has returned, so waiting for + // it waits for the round trip. A fixed sleep only guessed at how long that takes, and guessed + // from a developer's machine. + await Assertions.Expect(page.Locator(".hpm-event").Filter(new() { HasTextString = "interval set to" }).First) + .ToBeVisibleAsync(); // Reloading drops every scrap of component state, so what comes back can only have come // from the state provider. await page.ReloadAsync(new() { WaitUntil = WaitUntilState.NetworkIdle }); - await RowFor(page, TargetChecker).WaitForAsync(new() { Timeout = 30_000 }); + await WaitForTheBoardAsync(page); await RowFor(page, TargetChecker).ClickAsync(); - await page.Locator(".hpm-sel-name", new() { HasTextString = TargetChecker }) - .WaitForAsync(new() { Timeout = 10_000 }); + await Assertions.Expect(page.Locator(".hpm-sel-name")).ToHaveTextAsync(TargetChecker); - Assert.Equal("Every5Minutes", await page.Locator("#hpm-interval").InputValueAsync()); + await Assertions.Expect(page.Locator("#hpm-interval")).ToHaveValueAsync("Every5Minutes"); browser.AssertNoErrors(page); } @@ -183,15 +205,15 @@ public async Task ThemeToggle_SwitchesBothWays(ProviderSetup setup) await using var app = await SampleApp.StartAsync(setup, Ct); var page = await OpenDashboardAsync(app); - Assert.Equal("dark", await page.Locator(".healthie-dashboard").GetAttributeAsync("data-hpm")); + var board = page.Locator(".healthie-dashboard"); + + await Assertions.Expect(board).ToHaveAttributeAsync("data-hpm", "dark"); await page.Locator(".hpm-btn--theme").ClickAsync(); - await page.WaitForTimeoutAsync(600); - Assert.Equal("light", await page.Locator(".healthie-dashboard").GetAttributeAsync("data-hpm")); + await Assertions.Expect(board).ToHaveAttributeAsync("data-hpm", "light"); await page.Locator(".hpm-btn--theme").ClickAsync(); - await page.WaitForTimeoutAsync(600); - Assert.Equal("dark", await page.Locator(".healthie-dashboard").GetAttributeAsync("data-hpm")); + await Assertions.Expect(board).ToHaveAttributeAsync("data-hpm", "dark"); browser.AssertNoErrors(page); } From 9ee109295583b579bfae8561b8eddc9bde1b5b07 Mon Sep 17 00:00:00 2001 From: Ivan Vydrin Date: Thu, 30 Jul 2026 21:57:42 +0300 Subject: [PATCH 2/5] Use the select-and-wait helper the cron tests introduced It was added with one call site and left unused everywhere else, so four tests still clicked a row and then typed, clicked RUN NOW, or read the interval picker without waiting for the panel to be showing the checker they had just selected -- which is the exact failure its own remarks describe. The comment explaining why moves onto the helper, where it now covers every caller. --- tests/Healthie.Tests.E2E/BrowserFixture.cs | 3 ++- .../DashboardGroupingTests.cs | 21 +++++++------------ .../DashboardReadOnlyTests.cs | 8 +++---- tests/Healthie.Tests.E2E/DashboardTests.cs | 6 +++--- 4 files changed, 17 insertions(+), 21 deletions(-) diff --git a/tests/Healthie.Tests.E2E/BrowserFixture.cs b/tests/Healthie.Tests.E2E/BrowserFixture.cs index 48a8047..eac9a9a 100644 --- a/tests/Healthie.Tests.E2E/BrowserFixture.cs +++ b/tests/Healthie.Tests.E2E/BrowserFixture.cs @@ -1,5 +1,6 @@ using System.Collections.Concurrent; using Microsoft.Playwright; +using Xunit.Sdk; namespace Healthie.Tests.E2E; @@ -11,7 +12,7 @@ public sealed class BrowserFixture : IAsyncLifetime { private readonly ConcurrentDictionary> _errors = new(); private readonly ConcurrentDictionary _contexts = new(); - private readonly ConcurrentDictionary _owners = new(); + private readonly ConcurrentDictionary _owners = new(); private IPlaywright? _playwright; private IBrowser? _browser; diff --git a/tests/Healthie.Tests.E2E/DashboardGroupingTests.cs b/tests/Healthie.Tests.E2E/DashboardGroupingTests.cs index 4b0dd03..8f9ad6c 100644 --- a/tests/Healthie.Tests.E2E/DashboardGroupingTests.cs +++ b/tests/Healthie.Tests.E2E/DashboardGroupingTests.cs @@ -16,9 +16,6 @@ namespace Healthie.Tests.E2E; [Collection(nameof(BrowserCollection))] public class DashboardGroupingTests(BrowserFixture browser) : IAsyncDisposable { - /// Closes the pages this test opened, keeping a trace behind if it failed. - public async ValueTask DisposeAsync() => await browser.FinishCurrentTestAsync(); - private static CancellationToken Ct => TestContext.Current.CancellationToken; /// Grouped and tagged by the sample, so it exercises both concepts at once. @@ -67,6 +64,9 @@ private async Task OpenDashboardAsync(SampleApp app) return page; } + /// Closes the pages this test opened, keeping a trace behind if it failed. + public async ValueTask DisposeAsync() => await browser.FinishCurrentTestAsync(); + /// /// The defaults declared in code have to survive the whole way to the screen: seeded into /// state on the first run, read back, and rendered. @@ -178,10 +178,7 @@ public async Task Dashboard_CronEditor_AppliesAValidExpressionAndRefusesABadOne( await using var app = await SampleApp.StartAsync(setup, Ct); var page = await OpenDashboardAsync(app); - // Waiting on the panel's title, not on the box: the box is already visible for whichever - // checker was selected on load, so it tells you nothing about whether the click has landed. - await RowFor(page, CronChecker).ClickAsync(); - await Assertions.Expect(page.Locator(".hpm-sel-name")).ToHaveTextAsync(CronChecker); + await SelectCheckerAsync(page, CronChecker); var box = page.Locator("#hpm-cron"); var before = await box.InputValueAsync(); @@ -213,12 +210,10 @@ public async Task Dashboard_ForACronChecker_DisablesTheIntervalPicker(ProviderSe await using var app = await SampleApp.StartAsync(setup, Ct); var page = await OpenDashboardAsync(app); - await RowFor(page, CronChecker).ClickAsync(); - await Assertions.Expect(page.Locator(".hpm-sel-name")).ToHaveTextAsync(CronChecker); + await SelectCheckerAsync(page, CronChecker); await Assertions.Expect(page.Locator("#hpm-interval")).ToBeDisabledAsync(); - await RowFor(page, TargetChecker).ClickAsync(); - await Assertions.Expect(page.Locator(".hpm-sel-name")).ToHaveTextAsync(TargetChecker); + await SelectCheckerAsync(page, TargetChecker); await Assertions.Expect(page.Locator("#hpm-interval")).ToBeEnabledAsync(); browser.AssertNoErrors(page); @@ -327,7 +322,7 @@ public async Task Dashboard_AddingATag_ShowsItOnTheRowAndSurvivesAReload(Provide await using var app = await SampleApp.StartAsync(setup, Ct); var page = await OpenDashboardAsync(app); - await RowFor(page, TargetChecker).ClickAsync(); + await SelectCheckerAsync(page, TargetChecker); await page.GetByLabel("Add a tag").FillAsync("e2e-added"); await page.GetByRole(AriaRole.Button, new() { Name = "ADD" }).ClickAsync(); @@ -365,7 +360,7 @@ public async Task Dashboard_ExpandingTheEventLog_OpensItAndClosesAgain(ProviderS var page = await OpenDashboardAsync(app); // The log starts empty and fills as checks report in, so give it something to show. - await RowFor(page, TargetChecker).ClickAsync(); + await SelectCheckerAsync(page, TargetChecker); await page.GetByRole(AriaRole.Button, new() { Name = "RUN NOW" }).ClickAsync(); await page.Locator(".hpm-log-body .hpm-event").First.WaitForAsync(); diff --git a/tests/Healthie.Tests.E2E/DashboardReadOnlyTests.cs b/tests/Healthie.Tests.E2E/DashboardReadOnlyTests.cs index 37fbdf3..b1fdb08 100644 --- a/tests/Healthie.Tests.E2E/DashboardReadOnlyTests.cs +++ b/tests/Healthie.Tests.E2E/DashboardReadOnlyTests.cs @@ -1,4 +1,4 @@ -using Microsoft.Playwright; +using Microsoft.Playwright; namespace Healthie.Tests.E2E; @@ -19,9 +19,6 @@ namespace Healthie.Tests.E2E; [Collection(nameof(BrowserCollection))] public class DashboardReadOnlyTests(BrowserFixture browser) : IAsyncDisposable { - /// Closes the pages this test opened, keeping a trace behind if it failed. - public async ValueTask DisposeAsync() => await browser.FinishCurrentTestAsync(); - private static CancellationToken Ct => TestContext.Current.CancellationToken; private static readonly ProviderSetup Setup = new("Timer", UseCosmos: false); @@ -53,6 +50,9 @@ private async Task OpenDashboardAsync(SampleApp app) return page; } + /// Closes the pages this test opened, keeping a trace behind if it failed. + public async ValueTask DisposeAsync() => await browser.FinishCurrentTestAsync(); + [Fact] public async Task Dashboard_WithMutationsOff_HidesEveryControlThatChangesAChecker() { diff --git a/tests/Healthie.Tests.E2E/DashboardTests.cs b/tests/Healthie.Tests.E2E/DashboardTests.cs index 8863385..92d6595 100644 --- a/tests/Healthie.Tests.E2E/DashboardTests.cs +++ b/tests/Healthie.Tests.E2E/DashboardTests.cs @@ -10,9 +10,6 @@ namespace Healthie.Tests.E2E; [Collection(nameof(BrowserCollection))] public class DashboardTests(BrowserFixture browser) : IAsyncDisposable { - /// Closes the pages this test opened, keeping a trace behind if it failed. - public async ValueTask DisposeAsync() => await browser.FinishCurrentTestAsync(); - private static CancellationToken Ct => TestContext.Current.CancellationToken; /// @@ -82,6 +79,9 @@ private async Task OpenDashboardAsync(SampleApp app) internal static Task WaitForTheBoardAsync(IPage page) => Assertions.Expect(page.Locator(".hpm-row")).ToHaveCountAsync(CheckerCount, new() { Timeout = 30_000 }); + /// Closes the pages this test opened, keeping a trace behind if it failed. + public async ValueTask DisposeAsync() => await browser.FinishCurrentTestAsync(); + [Theory] [MemberData(nameof(Setups))] public async Task Dashboard_ListsEveryChecker_AndReportsNoBrowserErrors(ProviderSetup setup) From 1fba66c4b9ee3d4dfd2c2c1dc5e0f8d1ac6c690c Mon Sep 17 00:00:00 2001 From: Ivan Vydrin Date: Thu, 30 Jul 2026 22:05:51 +0300 Subject: [PATCH 3/5] Read counts from the constant now the board is waited for Three tests still opened by counting the rows on screen and comparing against that number later. With `WaitForTheBoardAsync` in front of them the number is known, so the reads go and the assertions state what the sample actually declares -- which is also what fails usefully if a row goes missing, rather than comparing one wrong number against itself. Also drops a stray BOM from two files and clears a page's captured console errors when its test finishes, alongside the context it belongs to; they were held for the whole run before. --- tests/Healthie.Tests.E2E/BrowserFixture.cs | 3 ++- tests/Healthie.Tests.E2E/DashboardGroupingTests.cs | 7 ++----- tests/Healthie.Tests.E2E/DashboardReadOnlyTests.cs | 2 +- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/Healthie.Tests.E2E/BrowserFixture.cs b/tests/Healthie.Tests.E2E/BrowserFixture.cs index eac9a9a..d875ed9 100644 --- a/tests/Healthie.Tests.E2E/BrowserFixture.cs +++ b/tests/Healthie.Tests.E2E/BrowserFixture.cs @@ -1,4 +1,4 @@ -using System.Collections.Concurrent; +using System.Collections.Concurrent; using Microsoft.Playwright; using Xunit.Sdk; @@ -87,6 +87,7 @@ public async Task FinishCurrentTestAsync() _contexts.TryRemove(page, out _); _owners.TryRemove(page, out _); + _errors.TryRemove(page, out _); if (failed) { diff --git a/tests/Healthie.Tests.E2E/DashboardGroupingTests.cs b/tests/Healthie.Tests.E2E/DashboardGroupingTests.cs index 8f9ad6c..12ffb8c 100644 --- a/tests/Healthie.Tests.E2E/DashboardGroupingTests.cs +++ b/tests/Healthie.Tests.E2E/DashboardGroupingTests.cs @@ -250,7 +250,6 @@ public async Task Dashboard_CollapsingAGroup_HidesItsRowsAndLeavesTheOthers(Prov await group.WaitForAsync(); var inside = await group.Locator(".hpm-row").CountAsync(); - var total = await page.Locator(".hpm-row").CountAsync(); Assert.True(inside > 0); await group.Locator(".hpm-group-head").ClickAsync(); @@ -258,7 +257,7 @@ public async Task Dashboard_CollapsingAGroup_HidesItsRowsAndLeavesTheOthers(Prov // Awaited rather than counted on the spot: the click is a round-trip to the server and back // before anything re-renders. await Assertions.Expect(group.Locator(".hpm-row")).ToHaveCountAsync(0); - await Assertions.Expect(page.Locator(".hpm-row")).ToHaveCountAsync(total - inside); + await Assertions.Expect(page.Locator(".hpm-row")).ToHaveCountAsync(DashboardTests.CheckerCount - inside); browser.AssertNoErrors(page); } @@ -273,10 +272,8 @@ public async Task Dashboard_FilteringByATag_ShowsOnlyTheCheckersCarryingIt(Provi await using var app = await SampleApp.StartAsync(setup, Ct); var page = await OpenDashboardAsync(app); - var total = await page.Locator(".hpm-row").CountAsync(); - await page.Locator(".hpm-tag-filter").SelectOptionAsync("tier-1"); - await Assertions.Expect(page.Locator(".hpm-row")).Not.ToHaveCountAsync(total); + await Assertions.Expect(page.Locator(".hpm-row")).Not.ToHaveCountAsync(DashboardTests.CheckerCount); Assert.True( await page.Locator(".hpm-row").CountAsync() > 0, diff --git a/tests/Healthie.Tests.E2E/DashboardReadOnlyTests.cs b/tests/Healthie.Tests.E2E/DashboardReadOnlyTests.cs index b1fdb08..4bba0aa 100644 --- a/tests/Healthie.Tests.E2E/DashboardReadOnlyTests.cs +++ b/tests/Healthie.Tests.E2E/DashboardReadOnlyTests.cs @@ -1,4 +1,4 @@ -using Microsoft.Playwright; +using Microsoft.Playwright; namespace Healthie.Tests.E2E; From 72689e921c2cee04ab634a7a7d0889f0375f9cc0 Mon Sep 17 00:00:00 2001 From: Ivan Vydrin Date: Thu, 30 Jul 2026 22:20:39 +0300 Subject: [PATCH 4/5] Read the disposed listener's count after disposal, not before it `OnceDisposed_ItStopsCounting` failed a full-suite run: expected 3, got 4. Not the feature -- the test. A `MeterListener` subscribes to the meter by name, so it counts every checker in the process, and the count it took before calling Dispose was overtaken by a check another test ran in the gap. The assertion then read that difference as a listener still attached. Taking the number after disposal makes the very thing under test the thing that freezes it, so concurrent traffic cannot move it -- while a Dispose that failed to detach still does, which is how this was checked: neutering Dispose turns the test red. --- tests/Healthie.Tests.Unit/MetricsInsightsTests.cs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/Healthie.Tests.Unit/MetricsInsightsTests.cs b/tests/Healthie.Tests.Unit/MetricsInsightsTests.cs index 20ede61..6b4edde 100644 --- a/tests/Healthie.Tests.Unit/MetricsInsightsTests.cs +++ b/tests/Healthie.Tests.Unit/MetricsInsightsTests.cs @@ -166,13 +166,20 @@ public async Task OnceDisposed_ItStopsCounting() using var checker = Checker(provider, PulseCheckerHealth.Healthy, "disposal-target"); await checker.TriggerAsync(Ct); - var before = metrics.Snapshot(Ct).Checks; - Assert.True(before > 0); + Assert.True(metrics.Snapshot(Ct).Checks > 0); metrics.Dispose(); + // Read after disposal, not before it. A listener subscribes to the meter by name, so it + // counts every checker in the process -- including the ones other tests are running at the + // same time. A count taken before the Dispose call can be overtaken by one of those in the + // gap, which is a difference the assertion below then reads as a listener still attached. + // Taken here the number is frozen by the very thing under test, and a Dispose that failed + // to detach still moves it. + var frozen = metrics.Snapshot(Ct).Checks; + await checker.TriggerAsync(Ct); - Assert.Equal(before, metrics.Snapshot(Ct).Checks); + Assert.Equal(frozen, metrics.Snapshot(Ct).Checks); } } From 8a121ee77bc055fca20465840568ccaf688c3424 Mon Sep 17 00:00:00 2001 From: Ivan Vydrin Date: Thu, 30 Jul 2026 22:31:18 +0300 Subject: [PATCH 5/5] Stop the release running both suites on one runner at once This is what actually made releases fail browser tests that were green on every pull request. `ci.yml` gives the unit suite and the browser suite a runner each; the release ran one solution-level `dotnet test`, which hands both to the same machine at the same time. Reproducing that locally failed four browser tests, and the traces say why: Connection disconnected with error 'WebSocket closed with status code: 1006 (no reason given).' The browser suite drives a Blazor Server circuit over a WebSocket while eleven hundred unit tests across two frameworks saturate the runner. A starved circuit closes, and from that moment nothing on the page can change -- so every assertion waits out its timeout and reports the element it was watching, which reads as a broken feature. No amount of retrying helps; the page is dead. Two sequential steps instead. Both suites still gate the release, they just no longer compete. Run separately the same machine passes both: 53/53 browser, 558/558 unit on each framework. A failing browser test now also prints the console errors it collected straight into the CI log, so the next dropped circuit says so in the run output rather than only in a downloaded trace. --- .github/workflows/publish.yml | 16 +++++++++++++++- tests/Healthie.Tests.E2E/BrowserFixture.cs | 13 ++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index a1e9ddd..8c3dd3c 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -115,9 +115,23 @@ jobs: # The whole suite, unit and end-to-end, gates the release: nothing is packed, tagged, or # published unless the dashboard itself is proven to work in a browser first. + # + # Run as two steps rather than one solution-level `dotnet test`, which is not a style + # preference. That form hands both suites to one runner at once, and the browser suite is then + # driving a Blazor Server circuit over a WebSocket while eleven hundred unit tests across two + # frameworks saturate the machine. A starved circuit closes with 1006, and every assertion on + # that page fails from then on however long it waits -- which is what made releases fail + # browser tests that were green on every pull request, where ci.yml gives them a runner each. - name: Test run: > - dotnet test + dotnet test tests/Healthie.Tests.Unit + --configuration ${{ env.CONFIGURATION }} + --no-build + --verbosity normal + + - name: Test (browser) + run: > + dotnet test tests/Healthie.Tests.E2E --configuration ${{ env.CONFIGURATION }} --no-build --verbosity normal diff --git a/tests/Healthie.Tests.E2E/BrowserFixture.cs b/tests/Healthie.Tests.E2E/BrowserFixture.cs index d875ed9..84ac672 100644 --- a/tests/Healthie.Tests.E2E/BrowserFixture.cs +++ b/tests/Healthie.Tests.E2E/BrowserFixture.cs @@ -87,10 +87,21 @@ public async Task FinishCurrentTestAsync() _contexts.TryRemove(page, out _); _owners.TryRemove(page, out _); - _errors.TryRemove(page, out _); + _errors.TryRemove(page, out var errors); if (failed) { + // Straight into the CI log, next to the failure, because the usual cause of a + // browser assertion running out of patience is not a slow page: it is a Blazor + // circuit that closed, after which nothing on that page can ever change. That reads + // as "the element never got the attribute" and costs an afternoon. + if (errors is { Count: > 0 }) + { + TestContext.Current.TestOutputHelper?.WriteLine( + "The browser reported errors before this failed:" + Environment.NewLine + + string.Join(Environment.NewLine, errors)); + } + Directory.CreateDirectory(TraceDirectory); var safe = string.Concat(name.Select(c => char.IsLetterOrDigit(c) || c is '-' or '_' ? c : '-'));