Stop releases failing browser tests that pass everywhere else

.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

.github/workflows/publish.yml

@@ -115,13 +115,40 @@ 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
+
+      # 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

tests/Healthie.Tests.E2E/BrowserFixture.cs

@@ -1,5 +1,6 @@
 using System.Collections.Concurrent;
 using Microsoft.Playwright;
+using Xunit.Sdk;
 
 namespace Healthie.Tests.E2E;
 
@@ -10,6 +11,8 @@ namespace Healthie.Tests.E2E;
 public sealed class BrowserFixture : IAsyncLifetime
 {
     private readonly ConcurrentDictionary<IPage, List<string>> _errors = new();
+    private readonly ConcurrentDictionary<IPage, IBrowserContext> _contexts = new();
+    private readonly ConcurrentDictionary<IPage, ITest> _owners = new();
     private IPlaywright? _playwright;
     private IBrowser? _browser;
 
@@ -41,9 +44,82 @@ public async ValueTask InitializeAsync()
     public async Task<IPage> 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;
     }
 
+    /// <summary>
+    /// Closes every page the current test opened, keeping a trace only if it failed.
+    /// </summary>
+    /// <remarks>
+    /// Kept only on failure because a trace is a few megabytes and fifty-three passing ones are worth
+    /// nothing. <see cref="TraceDirectory"/> is what CI uploads.
+    /// </remarks>
+    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 _);
+            _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 : '-'));
+                await context.Tracing.StopAsync(new() { Path = Path.Combine(TraceDirectory, $"{safe}.zip") });
+            }
+            else
+            {
+                await context.Tracing.StopAsync();
+            }
+
+            await context.CloseAsync();
+        }
+    }
+
+    /// <summary>Where failure traces are written, and what CI collects.</summary>
+    public static string TraceDirectory { get; } =
+        Path.Combine(AppContext.BaseDirectory, "playwright-traces");
+
     /// <summary>Records the console and page errors seen on a page, for <see cref="AssertNoErrors"/>.</summary>
     public void TrackErrors(IPage page, List<string> errors) => _errors[page] = errors;
 

tests/Healthie.Tests.E2E/DashboardGroupingTests.cs

@@ -14,7 +14,7 @@ namespace Healthie.Tests.E2E;
 /// is the test that would have caught it.
 /// </remarks>
 [Collection(nameof(BrowserCollection))]
-public class DashboardGroupingTests(BrowserFixture browser)
+public class DashboardGroupingTests(BrowserFixture browser) : IAsyncDisposable
 {
     private static CancellationToken Ct => TestContext.Current.CancellationToken;
 
@@ -60,10 +60,13 @@ private async Task<IPage> 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;
     }
 
+    /// <summary>Closes the pages this test opened, keeping a trace behind if it failed.</summary>
+    public async ValueTask DisposeAsync() => await browser.FinishCurrentTestAsync();
+
     /// <summary>
     /// 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.
@@ -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);
     }
 
@@ -177,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();
@@ -212,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);
@@ -254,15 +250,14 @@ 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();
 
         // 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);
     }
 
@@ -277,16 +272,19 @@ 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);
-
-        var shown = await page.Locator(".hpm-row").CountAsync();
-        Assert.True(shown > 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());
+        await Assertions.Expect(page.Locator(".hpm-row")).Not.ToHaveCountAsync(DashboardTests.CheckerCount);
+
+        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. 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 +304,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);
@@ -320,7 +319,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();
 
@@ -329,7 +328,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 +342,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);
     }
 
@@ -360,7 +357,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();
 

tests/Healthie.Tests.E2E/DashboardReadOnlyTests.cs

@@ -17,7 +17,7 @@ namespace Healthie.Tests.E2E;
 /// </para>
 /// </remarks>
 [Collection(nameof(BrowserCollection))]
-public class DashboardReadOnlyTests(BrowserFixture browser)
+public class DashboardReadOnlyTests(BrowserFixture browser) : IAsyncDisposable
 {
     private static CancellationToken Ct => TestContext.Current.CancellationToken;
 
@@ -46,10 +46,13 @@ private async Task<IPage> 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;
     }
 
+    /// <summary>Closes the pages this test opened, keeping a trace behind if it failed.</summary>
+    public async ValueTask DisposeAsync() => await browser.FinishCurrentTestAsync();
+
     [Fact]
     public async Task Dashboard_WithMutationsOff_HidesEveryControlThatChangesAChecker()
     {
@@ -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);
     }