Skip to content

Add packaged-app smoke test to CI - #33

Open
JosephMaynard wants to merge 1 commit into
masterfrom
ci/smoke-launch-test
Open

Add packaged-app smoke test to CI#33
JosephMaynard wants to merge 1 commit into
masterfrom
ci/smoke-launch-test

Conversation

@JosephMaynard

@JosephMaynard JosephMaynard commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Add a packaged-app smoke test to CI

This closes the process gap that let v0.20.0 ship a build that crashed on launch (Cannot find module 'zod') while CI was green.

Why it was missed

CI builds the package (electron-forge package) but never launches it, and the unit tests run against source with node_modules present, so neither can see a packaging fault (a dependency that is not bundled and fails to resolve in the packaged app).

What this adds

  • scripts/smoke-test-packaged.mjs: launches the actual built binary, waits for the control window to appear over the Chrome DevTools endpoint, and asserts the React root has mounted content. It fails (non-zero exit) if the process exits early or no window renders within 45s, and prints the app's own output to make a CI failure diagnosable. Cross-platform, so it also runs locally via npm run smoke:packaged.
  • CI: the Test job installs xvfb plus the core rendering libraries and runs the smoke test under a virtual display after the package build.

Had this been in place, the broken build would have failed CI instead of reaching a user.

Verified

Ran it locally against a macOS package (both the raw script and npm run smoke:packaged): it detects the rendered window and exits 0. Reasoned through the failure paths too: a crash-exit trips the early-exit check, and an error-dialog hang trips the timeout.

Note

This runs on the Linux runner only (the Test job). That is enough to catch this class of bug, which is platform-independent (module resolution / bundling). Testing the Windows and macOS packaged launches in CI would need those runners; happy to add if you want that belt-and-suspenders. Could also be added to the release workflow itself for a final gate before publishing.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Added an automated smoke test to verify that packaged applications launch successfully and render the expected interface.
    • Added support for running the packaged-app test in a virtual display environment during continuous integration.
    • Improved failure reporting with captured application output and startup diagnostics.

The released app crashed on launch (Cannot find module 'zod') while CI
stayed green, because CI builds the package but never launches it, and the
source-level tests run with node_modules present so they cannot see a
packaging fault.

Adds scripts/smoke-test-packaged.mjs, which launches the actual built
binary and confirms a window renders (main process starts, the renderer
mounts the React root), failing loudly with the app output otherwise. The
CI Test job runs it under xvfb after the package build. Also exposed as
npm run smoke:packaged for local use. Verified locally against a macOS
package.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vercel

vercel Bot commented Aug 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
playoverlay Ready Ready Preview Aug 8, 2026 1:36pm

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The pull request adds a packaged Electron application smoke test. The test validates binary startup and React rendering through CDP. CI installs Linux runtime libraries and runs the test under xvfb.

Changes

Packaged smoke testing

Layer / File(s) Summary
Packaged application launch and rendering validation
scripts/smoke-test-packaged.mjs, package.json
The new script discovers the packaged Electron binary, launches it with remote debugging, validates the PlayOverlay page and React root through CDP, reports failures, captures output, and cleans up the process. The smoke:packaged script exposes this test.
Continuous integration smoke execution
.github/workflows/ci.yml
CI installs required Linux GUI and runtime libraries and runs the packaged application under xvfb after packaging.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CI
  participant SmokeTest as smoke-test-packaged.mjs
  participant Electron as Packaged Electron binary
  participant CDP
  CI->>SmokeTest: Run smoke:packaged
  SmokeTest->>Electron: Launch with remote debugging
  SmokeTest->>CDP: Poll for PlayOverlay page
  CDP-->>SmokeTest: Return React root content
  SmokeTest->>Electron: Terminate process
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding a packaged-application smoke test to CI.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ci/smoke-launch-test

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)

54-55: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Add timeout-minutes to the smoke-test step.

The smoke test can hang instead of failing. evaluateInPage has no timeout on the CDP socket, and the fetch call in the poll loop has no timeout, so the 45-second internal budget is not guaranteed. Without a step timeout, a hung packaged app consumes the default 360-minute job limit. A step-level timeout gives a fast, clear failure.

🛠️ Proposed change
       - name: Smoke test that the packaged app launches
+        timeout-minutes: 5
         run: xvfb-run --auto-servernum node scripts/smoke-test-packaged.mjs
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml around lines 54 - 55, Add a step-level
timeout-minutes setting to the “Smoke test that the packaged app launches”
workflow step, keeping the existing xvfb-run command unchanged and using a short
limit that safely bounds hangs beyond the smoke test’s intended runtime.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/ci.yml:
- Around line 45-48: Update the “Install libraries for the packaged-app smoke
test” workflow step to replace libgtk-3-0 with libgtk-3-0t64 and add
libasound2t64 to the apt-get install package list, while preserving the existing
package installation flow.

In `@scripts/smoke-test-packaged.mjs`:
- Around line 77-85: Update the smoke-test polling loop to treat either a
non-null child.exitCode or child.signalCode as an early app termination, and
include the signal in the resulting error context. Bound the debug endpoint
fetch inside the loop with an AbortController timeout that cannot outlive the
remaining deadline, while preserving the existing polling and retry behavior.
- Around line 125-149: Update the child process startup flow around spawn and
waitForRenderedWindow to listen for child error events and reject the top-level
promise with that error, ensuring startup failures enter the existing SMOKE TEST
FAILED catch path and preserve captured output. Add rejection handling to the
main() invocation so failures outside the inner try are reported and set a
nonzero exit code rather than becoming uncaught or successful exits.
- Around line 43-69: Update evaluateInPage to use a CDP_TIMEOUT_MS constant of
5000 ms and bound both the WebSocket connection and Runtime.evaluate response
promises with timers. Add close handlers that reject pending promises when the
socket closes before completion, clear timers on settlement, and preserve
existing error/result handling so waitForRenderedWindow cannot be blocked
indefinitely.

---

Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 54-55: Add a step-level timeout-minutes setting to the “Smoke test
that the packaged app launches” workflow step, keeping the existing xvfb-run
command unchanged and using a short limit that safely bounds hangs beyond the
smoke test’s intended runtime.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 686fdeb2-221f-43f4-b724-193ed193f566

📥 Commits

Reviewing files that changed from the base of the PR and between ce2bc60 and 0054375.

📒 Files selected for processing (3)
  • .github/workflows/ci.yml
  • package.json
  • scripts/smoke-test-packaged.mjs

Comment thread .github/workflows/ci.yml
Comment on lines +45 to +48
- name: Install libraries for the packaged-app smoke test
run: |
sudo apt-get update
sudo apt-get install -y xvfb libgbm1 libnss3 libgtk-3-0

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Ubuntu 24.04 libgtk-3-0 renamed libgtk-3-0t64 apt install transitional package

💡 Result:

On Ubuntu 24.04 (Noble), the GTK 3 shared library package is effectively renamed from libgtk-3-0 to libgtk-3-0t64, and the non-t64 package is superseded/withdrawn in favor of the t64 one. 1) What libgtk-3-0t64 is - Ubuntu 24.04 provides a binary package named libgtk-3-0t64 as part of the gtk+3.0 source, described as the “GTK graphical user interface library” shared libraries package.[1][2] 2) Relationship to libgtk-3-0 (the “rename”) - The Ubuntu package metadata for libgtk-3-0t64 shows it replaces the older libgtk-3-0 for versions older than the new one (i.e., libgtk-3-0t64 “Replaces: libgtk-3-0 (<< 3.24.41-1.1)”) and breaks the older package (libgtk-3-0). This indicates the t64 package is intended to take over from the non-t64 package rather than coexist indefinitely.[3] - Launchpad shows the old libgtk-3-0 binary package is “Deleted” (superseded) for the Noble release pocket(s) as of April 2024, consistent with the transition to libgtk-3-0t64.[4] 3) Transitional-package behavior with apt - In general, Ubuntu t64 transitions are handled using packaging relationships (Replaces/Provides/Breaks) and/or repository metadata/quirks so that apt can swap installed packages to the correct t64 variant during upgrades.[5] - Practically, for a fresh install on 24.04, you should install libgtk-3-0t64 (not libgtk-3-0), because libgtk-3-0 is not the supported package for Noble; it was removed/superseded in the Noble release process.[4] Practical apt command for Ubuntu 24.04 - Use: sudo apt install libgtk-3-0t64 If you want to install both old and new names explicitly, note that “libgtk-3-0” may not be available as an install candidate in Noble (since it was deleted/superseded), while libgtk-3-0t64 is the available package.[4][1] Sources: Ubuntu package pages and Launchpad entries for Noble showing libgtk-3-0t64 exists and libgtk-3-0 is deleted/superseded, plus packaging metadata showing Replaces/Breaks relationships and an Ubuntu bug report describing apt transition handling for t64 libraries.[1][3][4][5]

Citations:


🏁 Script executed:

sed -n '1,90p' .github/workflows/ci.yml
printf '\nElectron and runner references:\n'
rg -n -i 'electron|ubuntu-latest|libasound|libgtk|xvfb|packaged-app|smoke' .github package.json package-lock.json yarn.lock pnpm-lock.yaml 2>/dev/null || true

Repository: JosephMaynard/playoverlay

Length of output: 23690


🌐 Web query:

Ubuntu 24.04 Noble packages libgtk-3-0t64 libasound2t64 Electron 43 Linux dependencies

💡 Result:

In Ubuntu 24.04 (Noble Numbat), the packages libgtk-3-0t64 and libasound2t64 are the required versions of the GTK 3 and ALSA libraries, respectively [1][2][3]. This change is part of a transition commonly referred to as the t64 transition, in which numerous libraries were rebuilt with 64-bit time_t values to ensure Year 2038 compatibility, particularly on 32-bit architectures [4][3]. Because of this transition, legacy package names such as libgtk-3-0 and libasound2 are no longer available in the Ubuntu 24.04 repositories, causing installation errors for applications that explicitly depend on those older names [5][3][6]. For Electron applications (including Electron 43) running on Ubuntu 24.04, you must update your dependency declarations and installation scripts to use the new t64-suffixed package names [5][7]. Recommended dependency adjustments for Ubuntu 24.04 include: - Replace libgtk-3-0 with libgtk-3-0t64 [5][3] - Replace libasound2 with libasound2t64 [5][3] - Replace libatspi2.0-0 with libatspi2.0-0t64 (if applicable) [6][8] - Replace libcups2 with libcups2t64 (if applicable) [6] If you are maintaining an Electron application and packaging it for distribution (e.g., via electron-builder), you must update the depends configuration in your build settings to point to these new package names to avoid unmet dependency errors during installation [9][8]. Electron 43 itself remains compatible with these libraries as long as the system requirements are correctly mapped to the updated package names [10].

Citations:


Update the Noble runtime package names. Replace libgtk-3-0 with libgtk-3-0t64; Ubuntu 24.04 does not provide the old package name. Include libasound2t64 for Electron’s ALSA runtime dependency.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml around lines 45 - 48, Update the “Install libraries
for the packaged-app smoke test” workflow step to replace libgtk-3-0 with
libgtk-3-0t64 and add libasound2t64 to the apt-get install package list, while
preserving the existing package installation flow.

Comment on lines +43 to +69
async function evaluateInPage(webSocketDebuggerUrl, expression) {
const ws = new WebSocket(webSocketDebuggerUrl);
try {
await new Promise((resolve, reject) => {
ws.onopen = resolve;
ws.onerror = () => reject(new Error('CDP socket error'));
});
return await new Promise((resolve, reject) => {
const id = 1;
ws.addEventListener('message', (event) => {
const data = JSON.parse(event.data);
if (data.id !== id) return;
if (data.error) reject(new Error(JSON.stringify(data.error)));
else resolve(data.result?.result?.value);
});
ws.send(
JSON.stringify({
id,
method: 'Runtime.evaluate',
params: { expression, returnByValue: true },
})
);
});
} finally {
ws.close();
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Add a timeout and a close handler so evaluateInPage cannot hang forever.

Neither promise in this function can be rejected by a stalled socket. If the WebSocket opens and the renderer then closes or never answers the Runtime.evaluate request, the returned promise never settles. waitForRenderedWindow awaits this call inside the poll loop, and it only checks deadline between iterations. The 45-second budget is then not enforced, and the CI job hangs until the workflow-level timeout instead of failing fast with the captured app output.

🛠️ Proposed fix: bound both phases
 async function evaluateInPage(webSocketDebuggerUrl, expression) {
   const ws = new WebSocket(webSocketDebuggerUrl);
+  const withTimeout = (promise, ms, message) =>
+    Promise.race([
+      promise,
+      new Promise((_, reject) =>
+        setTimeout(() => reject(new Error(message)), ms).unref?.()
+      ),
+    ]);
   try {
-    await new Promise((resolve, reject) => {
-      ws.onopen = resolve;
-      ws.onerror = () => reject(new Error('CDP socket error'));
-    });
-    return await new Promise((resolve, reject) => {
+    await withTimeout(
+      new Promise((resolve, reject) => {
+        ws.onopen = resolve;
+        ws.onerror = () => reject(new Error('CDP socket error'));
+        ws.onclose = () => reject(new Error('CDP socket closed'));
+      }),
+      CDP_TIMEOUT_MS,
+      'Timed out opening the CDP socket'
+    );
+    return await withTimeout(
+      new Promise((resolve, reject) => {
       const id = 1;
+      ws.onclose = () => reject(new Error('CDP socket closed before a reply'));
+      ws.onerror = () => reject(new Error('CDP socket error'));
       ws.addEventListener('message', (event) => {
         const data = JSON.parse(event.data);
         if (data.id !== id) return;
         if (data.error) reject(new Error(JSON.stringify(data.error)));
         else resolve(data.result?.result?.value);
       });
       ws.send(
         JSON.stringify({
           id,
           method: 'Runtime.evaluate',
           params: { expression, returnByValue: true },
         })
       );
-    });
+      }),
+      CDP_TIMEOUT_MS,
+      'Timed out waiting for the CDP reply'
+    );
   } finally {
     ws.close();
   }
 }

Declare the new constant near the other constants:

const CDP_TIMEOUT_MS = 5000;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async function evaluateInPage(webSocketDebuggerUrl, expression) {
const ws = new WebSocket(webSocketDebuggerUrl);
try {
await new Promise((resolve, reject) => {
ws.onopen = resolve;
ws.onerror = () => reject(new Error('CDP socket error'));
});
return await new Promise((resolve, reject) => {
const id = 1;
ws.addEventListener('message', (event) => {
const data = JSON.parse(event.data);
if (data.id !== id) return;
if (data.error) reject(new Error(JSON.stringify(data.error)));
else resolve(data.result?.result?.value);
});
ws.send(
JSON.stringify({
id,
method: 'Runtime.evaluate',
params: { expression, returnByValue: true },
})
);
});
} finally {
ws.close();
}
}
async function evaluateInPage(webSocketDebuggerUrl, expression) {
const ws = new WebSocket(webSocketDebuggerUrl);
const withTimeout = (promise, ms, message) =>
Promise.race([
promise,
new Promise((_, reject) =>
setTimeout(() => reject(new Error(message)), ms).unref?.()
),
]);
try {
await withTimeout(
new Promise((resolve, reject) => {
ws.onopen = resolve;
ws.onerror = () => reject(new Error('CDP socket error'));
ws.onclose = () => reject(new Error('CDP socket closed'));
}),
CDP_TIMEOUT_MS,
'Timed out opening the CDP socket'
);
return await withTimeout(
new Promise((resolve, reject) => {
const id = 1;
ws.onclose = () => reject(new Error('CDP socket closed before a reply'));
ws.onerror = () => reject(new Error('CDP socket error'));
ws.addEventListener('message', (event) => {
const data = JSON.parse(event.data);
if (data.id !== id) return;
if (data.error) reject(new Error(JSON.stringify(data.error)));
else resolve(data.result?.result?.value);
});
ws.send(
JSON.stringify({
id,
method: 'Runtime.evaluate',
params: { expression, returnByValue: true },
})
);
}),
CDP_TIMEOUT_MS,
'Timed out waiting for the CDP reply'
);
} finally {
ws.close();
}
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/smoke-test-packaged.mjs` around lines 43 - 69, Update evaluateInPage
to use a CDP_TIMEOUT_MS constant of 5000 ms and bound both the WebSocket
connection and Runtime.evaluate response promises with timers. Add close
handlers that reject pending promises when the socket closes before completion,
clear timers on settlement, and preserve existing error/result handling so
waitForRenderedWindow cannot be blocked indefinitely.

Comment on lines +77 to +85
while (Date.now() < deadline) {
if (child.exitCode !== null) {
throw new Error(
`App exited before a window appeared (code ${child.exitCode})`
);
}
try {
const res = await fetch(`http://127.0.0.1:${DEBUG_PORT}/json`);
const targets = await res.json();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Detect signal-terminated exits, and bound the fetch call.

Two gaps in the poll loop:

  1. child.exitCode stays null when the process is terminated by a signal. A packaged Electron app that dies from SIGSEGV or SIGABRT — a common symptom of a broken package on Linux — therefore never trips this check. The loop polls for the full 45 seconds and reports "Timed out waiting for the app window to render", which hides the real crash. Check child.signalCode as well.
  2. fetch has no timeout. If the debug endpoint accepts the connection and then stalls, this await outlives the deadline check, which is only evaluated between iterations.
🛠️ Proposed fix
   while (Date.now() < deadline) {
-    if (child.exitCode !== null) {
+    if (child.exitCode !== null || child.signalCode !== null) {
       throw new Error(
-        `App exited before a window appeared (code ${child.exitCode})`
+        `App exited before a window appeared (code ${child.exitCode}, signal ${child.signalCode})`
       );
     }
     try {
-      const res = await fetch(`http://127.0.0.1:${DEBUG_PORT}/json`);
+      const res = await fetch(`http://127.0.0.1:${DEBUG_PORT}/json`, {
+        signal: AbortSignal.timeout(POLL_INTERVAL_MS * 2),
+      });
       const targets = await res.json();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
while (Date.now() < deadline) {
if (child.exitCode !== null) {
throw new Error(
`App exited before a window appeared (code ${child.exitCode})`
);
}
try {
const res = await fetch(`http://127.0.0.1:${DEBUG_PORT}/json`);
const targets = await res.json();
while (Date.now() < deadline) {
if (child.exitCode !== null || child.signalCode !== null) {
throw new Error(
`App exited before a window appeared (code ${child.exitCode}, signal ${child.signalCode})`
);
}
try {
const res = await fetch(`http://127.0.0.1:${DEBUG_PORT}/json`, {
signal: AbortSignal.timeout(POLL_INTERVAL_MS * 2),
});
const targets = await res.json();
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/smoke-test-packaged.mjs` around lines 77 - 85, Update the smoke-test
polling loop to treat either a non-null child.exitCode or child.signalCode as an
early app termination, and include the signal in the resulting error context.
Bound the debug endpoint fetch inside the loop with an AbortController timeout
that cannot outlive the remaining deadline, while preserving the existing
polling and retry behavior.

Comment on lines +125 to +149
const child = spawn(binary, args, { stdio: ['ignore', 'pipe', 'pipe'] });
let output = '';
child.stdout.on('data', (d) => (output += d));
child.stderr.on('data', (d) => (output += d));

try {
const result = await waitForRenderedWindow(child);
console.log(
`OK: window "${result.title}" rendered (#root has ${result.rootChildren} children).`
);
process.exitCode = 0;
} catch (error) {
console.error(`SMOKE TEST FAILED: ${error.message}`);
if (output.trim()) {
console.error('--- app output ---');
console.error(output.trim());
console.error('------------------');
}
process.exitCode = 1;
} finally {
child.kill('SIGKILL');
}
}

main();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Handle the spawn error event and reject the top-level promise.

spawn reports a failure to start (for example EACCES on a packaged binary that lost its executable bit, or ENOENT on a stale path) through an asynchronous error event. This script registers no error listener, so Node re-throws it as an uncaught exception. The user then sees a raw stack trace instead of the SMOKE TEST FAILED message and the captured app output. main() is also invoked with no rejection handler, so any throw outside the try block exits with code 0 in some Node versions.

🛠️ Proposed fix
   const child = spawn(binary, args, { stdio: ['ignore', 'pipe', 'pipe'] });
   let output = '';
+  const spawnFailure = new Promise((_, reject) => {
+    child.once('error', (err) =>
+      reject(new Error(`Failed to launch the packaged app: ${err.message}`))
+    );
+  });
   child.stdout.on('data', (d) => (output += d));
   child.stderr.on('data', (d) => (output += d));
 
   try {
-    const result = await waitForRenderedWindow(child);
+    const result = await Promise.race([
+      waitForRenderedWindow(child),
+      spawnFailure,
+    ]);
-main();
+main().catch((error) => {
+  console.error(`SMOKE TEST FAILED: ${error.message}`);
+  process.exitCode = 1;
+});
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const child = spawn(binary, args, { stdio: ['ignore', 'pipe', 'pipe'] });
let output = '';
child.stdout.on('data', (d) => (output += d));
child.stderr.on('data', (d) => (output += d));
try {
const result = await waitForRenderedWindow(child);
console.log(
`OK: window "${result.title}" rendered (#root has ${result.rootChildren} children).`
);
process.exitCode = 0;
} catch (error) {
console.error(`SMOKE TEST FAILED: ${error.message}`);
if (output.trim()) {
console.error('--- app output ---');
console.error(output.trim());
console.error('------------------');
}
process.exitCode = 1;
} finally {
child.kill('SIGKILL');
}
}
main();
const child = spawn(binary, args, { stdio: ['ignore', 'pipe', 'pipe'] });
let output = '';
const spawnFailure = new Promise((_, reject) => {
child.once('error', (err) =>
reject(new Error(`Failed to launch the packaged app: ${err.message}`))
);
});
child.stdout.on('data', (d) => (output += d));
child.stderr.on('data', (d) => (output += d));
try {
const result = await Promise.race([
waitForRenderedWindow(child),
spawnFailure,
]);
console.log(
`OK: window "${result.title}" rendered (`#root` has ${result.rootChildren} children).`
);
process.exitCode = 0;
} catch (error) {
console.error(`SMOKE TEST FAILED: ${error.message}`);
if (output.trim()) {
console.error('--- app output ---');
console.error(output.trim());
console.error('------------------');
}
process.exitCode = 1;
} finally {
child.kill('SIGKILL');
}
}
main().catch((error) => {
console.error(`SMOKE TEST FAILED: ${error.message}`);
process.exitCode = 1;
});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/smoke-test-packaged.mjs` around lines 125 - 149, Update the child
process startup flow around spawn and waitForRenderedWindow to listen for child
error events and reject the top-level promise with that error, ensuring startup
failures enter the existing SMOKE TEST FAILED catch path and preserve captured
output. Add rejection handling to the main() invocation so failures outside the
inner try are reported and set a nonzero exit code rather than becoming uncaught
or successful exits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant