From e1181936015e2b4a451b69dc6782f24b13ab1054 Mon Sep 17 00:00:00 2001 From: Adriano Machado <60320+ammachado@users.noreply.github.com> Date: Fri, 18 Sep 2026 01:40:13 -0400 Subject: [PATCH] CAMEL-24485: windows-validator - stop killing slow-but-passing PowerShell suite runs The windows-validator job failed in 33 of 54 completed runs. 19 of those were surefire fork timeouts: WebsiteInstallTest$WindowsPowerShell takes 10-20 minutes on windows-latest (626s-1198s across passing runs) against camel-launcher's 1200s forkTimeout, so slow runs were killed. The "Connection reset" in the log is the Develocity event socket dying with the killed fork, not a network problem. - Raise the fork timeout to 2700s for this job's test step only, and cap the job at 60 minutes so a real hang still fails fast. - Upload surefire reports, and print one timing line per spawned process from the fixture; surefire streams that live, so it survives a fork kill where no XML report is written. - Pass TEMP/TMP through and point PSModuleAnalysisCachePath at one shared file: every test gives PowerShell a fresh LOCALAPPDATA, so each launch would rebuild its module analysis cache. - Route every helper process through a bounded WebsiteInstallerFixture.execute(): readAllBytes() before waitFor(30s) blocks until the pipe closes, so the timeout never applied (the POSIX assertVersionInstalled had no timeout at all). _Claude Code on behalf of Adriano Machado (@ammachado)_ _This was generated by an AI agent and may contain inaccuracies. Please verify before relying on it._ --- .../workflows/package-native-validation.yml | 18 +++++- .../jbang/launcher/WebsiteInstallTest.java | 58 ++++++------------- .../launcher/WebsiteInstallerFixture.java | 33 ++++++++++- 3 files changed, 68 insertions(+), 41 deletions(-) diff --git a/.github/workflows/package-native-validation.yml b/.github/workflows/package-native-validation.yml index 8a98b82c561d3..faeb7df8e9192 100644 --- a/.github/workflows/package-native-validation.yml +++ b/.github/workflows/package-native-validation.yml @@ -150,6 +150,8 @@ jobs: # ── Windows (install.ps1 unit tests) ────────────────────────────── windows-validator: runs-on: windows-latest + # Caps a genuine hang well below GitHub's 6h default; a passing run has taken up to ~25 minutes. + timeout-minutes: 60 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: @@ -184,6 +186,20 @@ jobs: - name: Run Windows installer unit tests shell: bash + # The PowerShell suite has taken 10-20 minutes on windows-latest, straddling camel-launcher's 1200s + # surefire fork timeout, so slow-but-passing runs were killed. Give this job's fork more room. run: | cd dsl/camel-jbang/camel-launcher - mvn -B -ntp test -pl . -Dtest=WebsiteInstallTest + mvn -B -ntp test -pl . -Dtest=WebsiteInstallTest -Dcamel.surefire.forkTimeout=2700 + + # Per-test timings for the PowerShell suite, which runs close to camel-launcher's surefire fork timeout. + # A fork killed on that timeout writes no XML report; the per-process timings the fixture prints to the + # build log cover that case. + - name: Archive surefire reports + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: windows-validator-surefire-reports + path: dsl/camel-jbang/camel-launcher/target/surefire-reports + retention-days: 7 + if-no-files-found: ignore diff --git a/dsl/camel-jbang/camel-launcher/src/test/java/org/apache/camel/dsl/jbang/launcher/WebsiteInstallTest.java b/dsl/camel-jbang/camel-launcher/src/test/java/org/apache/camel/dsl/jbang/launcher/WebsiteInstallTest.java index 0fbda6496e9d0..a514a7c108287 100644 --- a/dsl/camel-jbang/camel-launcher/src/test/java/org/apache/camel/dsl/jbang/launcher/WebsiteInstallTest.java +++ b/dsl/camel-jbang/camel-launcher/src/test/java/org/apache/camel/dsl/jbang/launcher/WebsiteInstallTest.java @@ -34,7 +34,6 @@ import java.util.Map; import java.util.Set; import java.util.UUID; -import java.util.concurrent.TimeUnit; import java.util.stream.Stream; import org.junit.jupiter.api.AfterEach; @@ -152,11 +151,8 @@ private static void assertVersionInstalled(Path home, String version) throws Exc Path versionDir = home.resolve(".local/share/camel-cli/versions/" + version); assertTrue(Files.isDirectory(versionDir), "expected version directory " + versionDir); - Process check = new ProcessBuilder("/bin/sh", shim.toString(), "version") - .redirectErrorStream(true) - .start(); - String output = new String(check.getInputStream().readAllBytes(), StandardCharsets.UTF_8); - check.waitFor(); + String output = WebsiteInstallerFixture.execute( + new ProcessBuilder("/bin/sh", shim.toString(), "version").redirectErrorStream(true)).stdout(); assertTrue(output.contains("Camel " + version), output); } @@ -672,9 +668,8 @@ void restoreUserPath() throws Exception { + "if ($path) { $kind = $k.GetValueKind('Path'); " + "$kept = ($path -split ';') | Where-Object { $_ -and (-not ($dirs -icontains $_)) }; " + "$k.SetValue('Path', ($kept -join ';'), $kind) } $k.Close() }"); - Process cleanup = new ProcessBuilder("powershell", "-NoProfile", "-Command", script.toString()) - .redirectErrorStream(true).start(); - cleanup.waitFor(30, TimeUnit.SECONDS); + WebsiteInstallerFixture.execute( + new ProcessBuilder("powershell", "-NoProfile", "-Command", script.toString()).redirectErrorStream(true)); } private WebsiteInstallerFixture.Result install( @@ -716,11 +711,8 @@ private static void assertVersionInstalled(Path home, String version) throws Exc assertTrue(Files.isDirectory(versionDir(home, version)), "expected version directory " + versionDir(home, version)); - Process check = new ProcessBuilder("cmd.exe", "/c", shim.toString(), "version") - .redirectErrorStream(true) - .start(); - String output = new String(check.getInputStream().readAllBytes(), StandardCharsets.UTF_8); - check.waitFor(30, TimeUnit.SECONDS); + String output = WebsiteInstallerFixture.execute( + new ProcessBuilder("cmd.exe", "/c", shim.toString(), "version").redirectErrorStream(true)).stdout(); assertTrue(output.contains("Camel " + version), output); } @@ -740,13 +732,10 @@ private void assertMaliciousZipRejected(Path temp, MaliciousArchive archiveBuild } private static String queryEnvironmentPath(String scope) throws Exception { - Process p = new ProcessBuilder( + return WebsiteInstallerFixture.execute(new ProcessBuilder( "powershell", "-NoProfile", "-Command", "[Environment]::GetEnvironmentVariable('Path','" + scope + "')") - .redirectErrorStream(true).start(); - String out = new String(p.getInputStream().readAllBytes(), StandardCharsets.UTF_8).trim(); - p.waitFor(30, TimeUnit.SECONDS); - return out; + .redirectErrorStream(true)).stdout().trim(); } private static long countOccurrencesCaseInsensitive(String path, String dir) { @@ -760,15 +749,12 @@ private static long countOccurrencesCaseInsensitive(String path, String dir) { // Reads HKCU\Environment\Path without expanding %VAR% references, returning "" when unset. private static String readUserPathRaw() throws Exception { - Process p = new ProcessBuilder( + return WebsiteInstallerFixture.execute(new ProcessBuilder( "powershell", "-NoProfile", "-Command", "$k = [Microsoft.Win32.Registry]::CurrentUser.OpenSubKey('Environment'); " + "if ($k) { [Console]::Out.Write($k.GetValue('Path', '', " - + "[Microsoft.Win32.RegistryValueOptions]::DoNotExpandEnvironmentNames)); $k.Close() }") - .start(); - String out = new String(p.getInputStream().readAllBytes(), StandardCharsets.UTF_8); - p.waitFor(30, TimeUnit.SECONDS); - return out; + + "[Microsoft.Win32.RegistryValueOptions]::DoNotExpandEnvironmentNames)); $k.Close() }")) + .stdout(); } // Writes HKCU\Environment\Path as a REG_EXPAND_SZ value; the value is passed via the environment @@ -781,21 +767,18 @@ private static void writeUserPathExpandable(String value) throws Exception { + "[Microsoft.Win32.RegistryValueKind]::ExpandString); $k.Close()") .redirectErrorStream(true); pb.environment().put("CAMEL_TEST_USERPATH", value); - Process p = pb.start(); - String out = new String(p.getInputStream().readAllBytes(), StandardCharsets.UTF_8); - if (!p.waitFor(30, TimeUnit.SECONDS) || p.exitValue() != 0) { - throw new IllegalStateException("failed to seed user PATH: " + out); + WebsiteInstallerFixture.Result r = WebsiteInstallerFixture.execute(pb); + if (r.exit() != 0) { + throw new IllegalStateException("failed to seed user PATH: " + r.stdout()); } } private static void deleteUserPath() throws Exception { - Process p = new ProcessBuilder( + WebsiteInstallerFixture.execute(new ProcessBuilder( "powershell", "-NoProfile", "-Command", "$k = [Microsoft.Win32.Registry]::CurrentUser.OpenSubKey('Environment', $true); " + "if ($k) { $k.DeleteValue('Path', $false); $k.Close() }") - .redirectErrorStream(true).start(); - p.getInputStream().readAllBytes(); - p.waitFor(30, TimeUnit.SECONDS); + .redirectErrorStream(true)); } @Test @@ -1039,14 +1022,11 @@ void forwardsArgumentsAndExitCodeThroughShim(@TempDir Path temp) throws Exceptio // ("bar baz") trips cmd.exe's quote-stripping rule (it only preserves quotes when there are // exactly two), which would unquote the space in "Apache Camel" and break the call. A bare // command name resolved through PATH+PATHEXT avoids that quirk and mirrors real usage. - Process argsProc = shimProcess(binDir, "echo-args", "foo", "bar baz").start(); - String argsOut = new String(argsProc.getInputStream().readAllBytes(), StandardCharsets.UTF_8); - argsProc.waitFor(30, TimeUnit.SECONDS); + String argsOut = WebsiteInstallerFixture.execute(shimProcess(binDir, "echo-args", "foo", "bar baz")) + .stdout(); assertTrue(argsOut.contains("foo") && argsOut.contains("bar baz"), argsOut); - Process exitProc = shimProcess(binDir, "exit-code", "7").start(); - exitProc.waitFor(30, TimeUnit.SECONDS); - assertEquals(7, exitProc.exitValue()); + assertEquals(7, WebsiteInstallerFixture.execute(shimProcess(binDir, "exit-code", "7")).exit()); } } diff --git a/dsl/camel-jbang/camel-launcher/src/test/java/org/apache/camel/dsl/jbang/launcher/WebsiteInstallerFixture.java b/dsl/camel-jbang/camel-launcher/src/test/java/org/apache/camel/dsl/jbang/launcher/WebsiteInstallerFixture.java index a94857d9b708a..996e2eddf74f1 100644 --- a/dsl/camel-jbang/camel-launcher/src/test/java/org/apache/camel/dsl/jbang/launcher/WebsiteInstallerFixture.java +++ b/dsl/camel-jbang/camel-launcher/src/test/java/org/apache/camel/dsl/jbang/launcher/WebsiteInstallerFixture.java @@ -64,6 +64,8 @@ final class WebsiteInstallerFixture implements AutoCloseable { private static final String KEY_ALIAS = "camel-installer-test"; private static final String DEFAULT_BASE_VERSION = "9.9.9"; private static final Duration PROCESS_TIMEOUT = Duration.ofSeconds(60); + private static final Path POWERSHELL_MODULE_ANALYSIS_CACHE + = Path.of("target", "powershell", "ModuleAnalysisCache").toAbsolutePath(); record Result(int exit, String stdout, String stderr) { } @@ -446,6 +448,20 @@ Result run(List command, Map env) throws Exception { if (pathExt != null) { pb.environment().put("PATHEXT", pathExt); } + // Without TEMP/TMP, .NET falls back to USERPROFILE (the fresh per-test home) for the scratch + // files Add-Type's C# compile writes on every install.ps1 run. install.ps1 itself never stages + // anything under the temp directory, so passing the runner's through does not affect isolation. + for (String name : List.of("TEMP", "TMP")) { + String value = System.getenv(name); + if (value != null) { + pb.environment().put(name, value); + } + } + // PowerShell keeps its module analysis cache under LOCALAPPDATA, which every test points at a + // fresh home, so each powershell.exe launch would rebuild it from scratch when auto-loading + // Expand-Archive / Get-FileHash. Share one cache file across the whole test run instead. + Files.createDirectories(POWERSHELL_MODULE_ANALYSIS_CACHE.getParent()); + pb.environment().put("PSModuleAnalysisCachePath", POWERSHELL_MODULE_ANALYSIS_CACHE.toString()); } pb.environment().putAll(env); String home = env.get("HOME"); @@ -454,7 +470,16 @@ Result run(List command, Map env) throws Exception { // wherever the test JVM happens to be running from. pb.directory(Path.of(home).toFile()); } + return execute(pb); + } + /** + * Runs {@code pb} with stdout and stderr drained on background threads, so a chatty or stuck child can never block + * the test past {@link #PROCESS_TIMEOUT}: reading a stream to EOF before {@code waitFor} would wait for as long as + * the child (or any grandchild inheriting its handles) keeps the pipe open. + */ + static Result execute(ProcessBuilder pb) throws Exception { + long start = System.nanoTime(); Process process = pb.start(); ExecutorService collectors = Executors.newFixedThreadPool(2); try { @@ -463,7 +488,7 @@ Result run(List command, Map env) throws Exception { process.getOutputStream().close(); if (!process.waitFor(PROCESS_TIMEOUT.toSeconds(), TimeUnit.SECONDS)) { process.destroyForcibly(); - throw new IllegalStateException("installer did not exit in time"); + throw new IllegalStateException("process did not exit in time: " + pb.command()); } String out = new String(await(stdout), StandardCharsets.UTF_8); String err = new String(await(stderr), StandardCharsets.UTF_8); @@ -474,6 +499,12 @@ Result run(List command, Map env) throws Exception { } collectors.shutdownNow(); collectors.awaitTermination(10, TimeUnit.SECONDS); + // Surefire streams test stdout to the build log as it happens, so these timings survive even when + // the fork is killed by forkedProcessTimeoutInSeconds and no XML report is ever written. + String command = String.join(" ", pb.command()); + System.out.printf("[WebsiteInstallerFixture] %d ms: %s%n", + TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start), + command.length() > 160 ? command.substring(0, 160) + "..." : command); } }