From 3a4f1586a558e3910968c5fb7f259f45e5eef564 Mon Sep 17 00:00:00 2001
From: Andrew Polk <andrew_polk@sil.org>
Date: Fri, 17 Jul 2026 10:12:19 -0700
Subject: [PATCH] Test hygiene: no CDP port in unit-test WebView2s; reset
 Program statics
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two issues found while investigating CI failures in the upload
integration tests (whose actual cause is server-side: the bloomlibrary
API's upload-start endpoint intermittently returns a bare 500 for
env=unit-test — reproducible with curl, nothing to do with client
code):

- WebView2Browser passed --remote-debugging-port (BloomServer http
  port + 2) to every WebView2 whenever a BloomServer was listening in
  the process, including in unit tests. Nothing attaches to a test-run
  WebView2, and on a CI machine that also runs real Bloom instances
  the port can already be taken, which breaks WebView2 startup and
  fails whatever test needed the browser. Now skipped when running
  unit tests.

- ProgramTests and ShellTests called
  Program.ParseStartupPortArguments("--automation") and left
  StartupAutomation=true in the Program statics for every later
  fixture in the run, making BloomServer print BLOOM_AUTOMATION_READY
  banners in unrelated tests' output. Both fixtures now re-parse empty
  args in a TearDown, which resets all the startup statics.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 src/BloomExe/WebView2Browser.cs |  6 +++++-
 src/BloomTests/ProgramTests.cs  | 13 +++++++++++++
 src/BloomTests/ShellTests.cs    | 11 +++++++++++
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/src/BloomExe/WebView2Browser.cs b/src/BloomExe/WebView2Browser.cs
index b66b1f43f53b..fb23e879c29f 100644
--- a/src/BloomExe/WebView2Browser.cs
+++ b/src/BloomExe/WebView2Browser.cs
@@ -396,9 +396,13 @@ private async Task InitWebView()
             {
                 additionalBrowserArgs += " --accept-lang=" + _uiLanguageOfThisRun;
             }
-            if (RemoteDebuggingPort.HasValue)
+            if (RemoteDebuggingPort.HasValue && !Program.RunningUnitTests)
             {
                 // Expose a CDP endpoint so Playwright and other automation can attach to the real Bloom WebView2 surface.
+                // NOT in unit tests: nothing attaches to a test-run WebView2, and on a busy CI machine the port
+                // (BloomServer's http port + 2) can already be held by a real Bloom instance running beside the
+                // tests, in which case Chromium's fight over the port breaks WebView2 startup and fails the test
+                // (seen as flaky BookUploadAndDownloadTests upload failures on agents also running Bloom automation).
                 additionalBrowserArgs += $" --remote-debugging-port={RemoteDebuggingPort.Value} "; // allow external inspector connect
             }
             if (featuresToDisable.Count > 0)
diff --git a/src/BloomTests/ProgramTests.cs b/src/BloomTests/ProgramTests.cs
index 5a721d9ec66e..afea45f47f3a 100644
--- a/src/BloomTests/ProgramTests.cs
+++ b/src/BloomTests/ProgramTests.cs
@@ -6,6 +6,19 @@ namespace BloomTests
     [TestFixture]
     public class ProgramTests
     {
+        /// <summary>
+        /// ParseStartupPortArguments stores its results in Program statics (StartupAutomation etc.)
+        /// which live for the rest of the test run. Re-parse empty args after each test to restore
+        /// the defaults; the method resets all of them on entry. Without this, the "--automation"
+        /// tests here left StartupAutomation=true for every later fixture, which (among other
+        /// things) made BloomServer print automation banners in unrelated tests' output.
+        /// </summary>
+        [TearDown]
+        public void TearDown()
+        {
+            Program.ParseStartupPortArguments(System.Array.Empty<string>(), out _);
+        }
+
         [Test]
         public void ParseStartupPortArguments_RemovesPortsAndStoresExplicitValues()
         {
diff --git a/src/BloomTests/ShellTests.cs b/src/BloomTests/ShellTests.cs
index ffb8d32a999d..ac6f63674560 100644
--- a/src/BloomTests/ShellTests.cs
+++ b/src/BloomTests/ShellTests.cs
@@ -7,6 +7,17 @@ namespace BloomTests
     [TestFixture]
     public class ShellTests
     {
+        /// <summary>
+        /// ParseStartupPortArguments writes into Program statics that live for the rest of the
+        /// test run; re-parse empty args after each test to restore the defaults (the method
+        /// resets them all on entry). Same rationale as the ProgramTests TearDown.
+        /// </summary>
+        [TearDown]
+        public void TearDown()
+        {
+            Program.ParseStartupPortArguments(Array.Empty<string>(), out _);
+        }
+
         [Test]
         public void ShouldShowPortSummaryInWindowTitle_TracksAutomationFlag()
         {