test(playwright): drop hollow SMTP restore, make render assert web-first (review fix)

tests/ZB.MOM.WW.ScadaBridge.CentralUI.PlaywrightTests/Notifications/SmtpConfigTests.cs

@@ -22,12 +22,15 @@ namespace ZB.MOM.WW.ScadaBridge.CentralUI.PlaywrightTests.Notifications;
 ///
 /// <para>
 /// <see cref="SmtpEdit_NoopSave_ShowsSavedToast"/> is the only mutating fact, and is
-/// gated on safety. The page's <c>StartEdit</c> handler loads the stored credential into
+/// gated on safety. Its sole safety mechanism is that the Save is a genuine no-op: the
-/// the form's <c>_credentials</c> field (<c>_credentials = smtp.Credentials;</c>), so
+/// page's <c>StartEdit</c> handler reloads EVERY field into the form
-/// re-saving an Edit form with every field untouched rewrites the SAME stored values — a
+/// (<c>_host/_port/_authType/_tlsMode/_credentials/_fromAddress</c>, including
-/// true no-op. The fact only proceeds when a config already exists (probed via the CLI);
+/// <c>_credentials = smtp.Credentials;</c>), so saving an untouched Edit form rewrites the
-/// it snapshots Host/Port/AuthType/TLS/From as belt-and-braces and best-effort restores
+/// SAME stored values — zero net change. No CLI restore is used: the masked credential is
-/// them afterwards. When no config exists there is nothing to edit, so it skips.
+/// 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).
 /// </para>
 /// </summary>
 [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()
+        // Web-first: assert the page resolved to EITHER a config card (Credentials '(stored)' /
-                    || await page.Locator("text=(not set)").IsVisibleAsync();
+        // '(not set)') OR the empty-state, with retry so a slow Blazor init does not spuriously
-        var emptyShown = await page.Locator("text=No SMTP configuration set.").IsVisibleAsync();
+        // fail. A union locator + single expectation replaces the point-in-time boolean OR.
-        Assert.True(cfgShown || emptyShown, "Expected an SMTP config card or the empty-state.");
+        await Assertions.Expect(
+            page.GetByText(new System.Text.RegularExpressions.Regex(@"\(stored\)|\(not set\)|No SMTP configuration set\."))
+                .First
+        ).ToBeVisibleAsync(new() { Timeout = 10_000 });
     }
 
     /// <summary>
@@ -114,12 +120,14 @@ public class SmtpConfigTests
     /// 'SMTP configuration saved.' toast appears.
     ///
     /// <para>
-    /// This is a true no-op only because the page's <c>StartEdit</c> handler loads the stored
+    /// The no-op save is the SOLE safety mechanism, and it is genuinely sufficient: the page's
-    /// credential into the form's password field (<c>_credentials = smtp.Credentials;</c>) —
+    /// <c>StartEdit</c> handler reloads every field into the form
-    /// so an untouched Save rewrites the SAME secret, not an empty one. As belt-and-braces the
+    /// (<c>_host/_port/_authType/_tlsMode/_credentials/_fromAddress</c>, including
-    /// fact snapshots Host/Port/AuthType/TLS/From via the CLI and best-effort restores them
+    /// <c>_credentials = smtp.Credentials;</c>), so an untouched Save rewrites the SAME stored
-    /// afterwards. When no config exists there is nothing to edit and the fact skips (SMTP
+    /// values — the credential included — for zero net change. No CLI restore is used: the
-    /// config has no delete verb, so we never create one we could not restore).
+    /// 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).
     /// </para>
     /// </summary>
     [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
+        // Open the first card's Edit form. StartEdit reloads EVERY field (including the stored
-        // stored credential), so re-saving untouched is a true no-op.
+        // 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.
-            }
-        }
     }
 }