From 4cfe950232d33bf7cb72b0b2cbd9865b26cede1a Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 6 Jun 2026 15:00:08 -0400 Subject: [PATCH] test(playwright): drop hollow SMTP restore, make render assert web-first (review fix) --- .../Notifications/SmtpConfigTests.cs | 76 +++++++++---------- 1 file changed, 35 insertions(+), 41 deletions(-) diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.PlaywrightTests/Notifications/SmtpConfigTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.PlaywrightTests/Notifications/SmtpConfigTests.cs index b988d51d..ab36b2df 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.PlaywrightTests/Notifications/SmtpConfigTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.PlaywrightTests/Notifications/SmtpConfigTests.cs @@ -22,12 +22,15 @@ namespace ZB.MOM.WW.ScadaBridge.CentralUI.PlaywrightTests.Notifications; /// /// /// is the only mutating fact, and is -/// gated on safety. The page's StartEdit handler loads the stored credential into -/// the form's _credentials field (_credentials = smtp.Credentials;), so -/// re-saving an Edit form with every field untouched rewrites the SAME stored values — a -/// true no-op. The fact only proceeds when a config already exists (probed via the CLI); -/// it snapshots Host/Port/AuthType/TLS/From as belt-and-braces and best-effort restores -/// them afterwards. When no config exists there is nothing to edit, so it skips. +/// gated on safety. Its sole safety mechanism is that the Save is a genuine no-op: the +/// page's StartEdit handler reloads EVERY field into the form +/// (_host/_port/_authType/_tlsMode/_credentials/_fromAddress, including +/// _credentials = smtp.Credentials;), so saving an untouched Edit form rewrites the +/// SAME stored values — zero net change. No CLI restore is used: the masked credential is +/// never exposed (un-snapshottable), and the save changes nothing, so a restore would add +/// risk without benefit. The fact only proceeds when a config already exists (probed via +/// the CLI); when none exists there is nothing to edit, so it skips — we never CREATE a +/// config we could not delete (SMTP config has no delete verb). /// /// [Collection("Playwright")] @@ -103,10 +106,13 @@ public class SmtpConfigTests await Assertions.Expect(page.Locator("h4:has-text('SMTP Configuration')")).ToBeVisibleAsync(); - var cfgShown = await page.Locator("text=(stored)").IsVisibleAsync() - || await page.Locator("text=(not set)").IsVisibleAsync(); - var emptyShown = await page.Locator("text=No SMTP configuration set.").IsVisibleAsync(); - Assert.True(cfgShown || emptyShown, "Expected an SMTP config card or the empty-state."); + // Web-first: assert the page resolved to EITHER a config card (Credentials '(stored)' / + // '(not set)') OR the empty-state, with retry so a slow Blazor init does not spuriously + // fail. A union locator + single expectation replaces the point-in-time boolean OR. + await Assertions.Expect( + page.GetByText(new System.Text.RegularExpressions.Regex(@"\(stored\)|\(not set\)|No SMTP configuration set\.")) + .First + ).ToBeVisibleAsync(new() { Timeout = 10_000 }); } /// @@ -114,12 +120,14 @@ public class SmtpConfigTests /// 'SMTP configuration saved.' toast appears. /// /// - /// This is a true no-op only because the page's StartEdit handler loads the stored - /// credential into the form's password field (_credentials = smtp.Credentials;) — - /// so an untouched Save rewrites the SAME secret, not an empty one. As belt-and-braces the - /// fact snapshots Host/Port/AuthType/TLS/From via the CLI and best-effort restores them - /// afterwards. When no config exists there is nothing to edit and the fact skips (SMTP - /// config has no delete verb, so we never create one we could not restore). + /// The no-op save is the SOLE safety mechanism, and it is genuinely sufficient: the page's + /// StartEdit handler reloads every field into the form + /// (_host/_port/_authType/_tlsMode/_credentials/_fromAddress, including + /// _credentials = smtp.Credentials;), so an untouched Save rewrites the SAME stored + /// values — the credential included — for zero net change. No CLI restore is used: the + /// masked credential is never exposed so it could not be snapshotted/restored anyway, and + /// the save mutates nothing. When no config exists there is nothing to edit and the fact + /// skips (SMTP config has no delete verb, so we never create one we could not delete). /// /// [SkippableFact] @@ -127,50 +135,36 @@ public class SmtpConfigTests { Skip.IfNot(await ClusterAvailability.IsAvailableAsync(), ClusterAvailability.SkipReason); - // Probe whether a config exists; if not, there is nothing safe to edit. + // Probe whether a config exists; if not, there is nothing safe to edit. We never + // CREATE a config here because SMTP config has no delete verb (we could not clean up). using var listDoc = await CliRunner.RunJsonAsync("notification", "smtp", "list"); var configs = listDoc.RootElement; Skip.If( configs.ValueKind != JsonValueKind.Array || configs.GetArrayLength() == 0, "No SMTP configuration exists to no-op-edit; SMTP config has no delete verb, " + - "so we never create one we cannot restore. Validation gate is covered by " + + "so we never create one we cannot delete. Validation gate is covered by " + "SmtpForm_MissingRequired_ShowsInlineError."); - // Snapshot the first config (belt-and-braces restore target). - var first = configs[0]; - var snapHost = first.GetProperty("host").GetString(); - var page = await _pw.NewAuthenticatedPageAsync(); await page.GotoAsync($"{PlaywrightFixture.BaseUrl}{SmtpUrl}"); await page.WaitForLoadStateAsync(LoadState.NetworkIdle); await Assertions.Expect(page.Locator("h4:has-text('SMTP Configuration')")).ToBeVisibleAsync(); - // Open the first card's Edit form. StartEdit pre-fills every field (including the - // stored credential), so re-saving untouched is a true no-op. + // Open the first card's Edit form. StartEdit reloads EVERY field (including the stored + // credential, _credentials = smtp.Credentials), so re-saving untouched rewrites the + // identical values — a true no-op. This is the sole safety mechanism: no CLI restore is + // used, because the masked credential is never exposed (un-snapshottable) and the save + // changes nothing, so a restore would add risk without benefit. await page.Locator("button.btn-outline-primary.btn-sm:has-text('Edit')").First.ClickAsync(); await Assertions.Expect(page.Locator("button.btn-success:has-text('Save')")).ToBeVisibleAsync(); - // Change NOTHING; click Save. + // Change NOTHING; click Save. StartEdit reloaded all fields, so this persists identical + // values — zero net change. await page.Locator("button.btn-success:has-text('Save')").ClickAsync(); await Assertions - .Expect(page.Locator(".toast:has-text('SMTP configuration saved.')")) + .Expect(page.Locator(".toast", new() { HasText = "SMTP configuration saved." })) .ToHaveCountAsync(1, new() { Timeout = 15_000 }); - - // Belt-and-braces: best-effort CLI restore of the snapshotted core fields. The no-op - // save should already have left them identical; this guards against any drift. - if (!string.IsNullOrEmpty(snapHost)) - { - try - { - await CliRunner.RunAsync("notification", "smtp", "update", "--host", snapHost); - } - catch - { - // Best-effort restore — the UI no-op save is the primary guarantee that the - // config is unchanged; do not fail the test on a restore hiccup. - } - } } }