Skip to content

Make the puppeteer test suite run headless with no local server #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
blogcastAI merged 1 commit into from
Aug 3, 2026
+41 −7
Merged

Make the puppeteer test suite run headless with no local server #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
# Continuous integration for Able Player
#
# Runs ESLint and the jsdom Jest project on every push and pull request.
# The puppeteer Jest project (validate.test.cjs) needs a demo server on
# localhost:8000 and a headed browser, so it is not run here yet — see
# jest.config.cjs. Build artifacts are compiled to confirm Grunt + Rollup
# succeed, but are never committed (per contributing.md).
# Runs ESLint and both Jest projects (jsdom + puppeteer) on every push and
# pull request. The puppeteer project runs headless with request
# interception, so no demo server is needed. Build artifacts are compiled
# to confirm Grunt + Rollup succeed, but are never committed (per
# contributing.md).
name: CI

on:
Expand Down Expand Up @@ -40,6 +40,20 @@ jobs:
- run: npm ci
- run: npx jest --selectProjects jsdom

test-browser:
name: Jest (puppeteer, headless)
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-node@v4
with:
node-version: 22
cache: npm
Comment on lines +47 to +51

@coderabbitai coderabbitai Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Pin the actions and disable checkout credential persistence.

Lines [47-51] use unpinned action references. actions/checkout also retains credentials in the local repository configuration. Pin both actions to approved full commit SHAs and set persist-credentials: false.

Proposed hardening 
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@<approved-full-commit-sha>
+        with:
+          persist-credentials: false
-      - uses: actions/setup-node@v4
+      - uses: actions/setup-node@<approved-full-commit-sha>
🧰 Tools
🪛 zizmor (1.28.0)

[warning] 47-47: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

[error] 47-47: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

[error] 48-48: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🤖 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 47 - 51, Update the workflow steps
using actions/checkout and actions/setup-node to reference approved full commit
SHAs instead of version tags, and add persist-credentials: false to the checkout
step’s with configuration. Preserve the existing Node.js version and npm cache
settings.

Source: Linters/SAST tools

- run: npm ci
# validate.test.cjs loads build/test/validate.umd.js
- run: npm run build
- run: npx jest --selectProjects puppeteer

build:
name: Build (Grunt + Rollup)
runs-on: ubuntu-latest
Expand Down
6 changes: 5 additions & 1 deletion jest-puppeteer.config.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
// jest-puppeteer.config.js
// Headless by default so `npm test` runs unattended (locally and in CI).
// Set HEADFUL=1 to watch the browser while debugging.
module.exports = {
launch: {
headless: false, // Set to true to run tests in headless mode
headless: process.env.HEADFUL ? false : true,

@coderabbitai coderabbitai Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Treat only HEADFUL=1 as headed mode.

Line [6] checks whether HEADFUL is present. Therefore, HEADFUL=0 and HEADFUL=false also disable headless mode. Compare the value with "1" to preserve unattended execution.

Proposed fix 
-    headless: process.env.HEADFUL ? false : true,
+    headless: process.env.HEADFUL !== "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
headless: process.env.HEADFUL ? false : true,
headless: process.env.HEADFUL !== "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 `@jest-puppeteer.config.cjs` at line 6, Update the headless option in the
Puppeteer configuration to disable headless mode only when process.env.HEADFUL
equals "1"; treat unset, "0", and "false" values as headless execution.

// Chromium's sandbox is unavailable in most CI containers.
args: process.env.CI ? ["--no-sandbox", "--disable-setuid-sandbox"] : [],
},
};
18 changes: 17 additions & 1 deletion scripts/__tests__/validate.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,23 @@ const path = require("path");
*/
describe("validate.js tests", () => {
beforeAll(async () => {
await page.goto("http://localhost:8000"); // Replace with your test URL
// The suite needs a real http(s) origin (isProtocolSafe resolves
// relative URLs against window.location.origin, which is opaque on
// about:blank), but no actual server: intercept the navigation and
// fulfill it with an empty page.
await page.setRequestInterception(true);
page.on("request", (request) => {
if (request.url().startsWith("http://ableplayer.test/")) {
request.respond({
status: 200,
contentType: "text/html",
body: "<!doctype html><html><head></head><body></body></html>",
});
Comment on lines +17 to +22

@coderabbitai coderabbitai Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Restrict the synthetic response to the navigation URL.

Lines [17-22] match every path below http://ableplayer.test/, not only the navigation at /. A same-origin script, stylesheet, image, or fetch then receives an HTML document with status 200. Match the exact URL used by page.goto, or provide resource-specific fixtures.

Proposed fix 
-      if (request.url().startsWith("http://ableplayer.test/")) {
+      if (request.url() === "http://ableplayer.test/") {
📝 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
if (request.url().startsWith("http://ableplayer.test/")) {
request.respond({
status: 200,
contentType: "text/html",
body: "<!doctype html><html><head></head><body></body></html>",
});
if (request.url() === "http://ableplayer.test/") {
request.respond({
status: 200,
contentType: "text/html",
body: "<!doctype html><html><head></head><body></body></html>",
});
🤖 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/__tests__/validate.test.cjs` around lines 17 - 22, Update the request
interception condition in the test setup to match only the exact navigation URL
used by page.goto, rather than every URL prefixed with http://ableplayer.test/.
Preserve the existing synthetic HTML response for the root navigation while
allowing same-origin scripts, stylesheets, images, and fetches to proceed or use
resource-specific fixtures.

} else {
request.continue();
}
});
await page.goto("http://ableplayer.test/");
Comment on lines +15 to +27

@coderabbitai coderabbitai Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add regression coverage for the intercepted origin.

Lines [15-27] introduce a new browser fixture but do not assert its contract. Capture the navigation response and assert status 200. Add a test that checks window.location.origin is exactly http://ableplayer.test.

As per path instructions, scripts/__tests__/** test expansion is the maintainer's stated top priority; add coverage for this new browser setup.

Proposed test coverage 
-    await page.goto("http://ableplayer.test/");
+    const response = await page.goto("http://ableplayer.test/");
+    expect(response?.status()).toBe(200);

+test("uses the intercepted HTTP origin", async () => {
+  expect(await page.evaluate(() => window.location.origin)).toBe(
+    "http://ableplayer.test",
+  );
+});
🤖 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/__tests__/validate.test.cjs` around lines 15 - 27, Extend the browser
fixture around page.goto to capture the navigation response and assert its
status is 200, then add a test asserting window.location.origin equals
http://ableplayer.test. Keep the existing request interception behavior
unchanged and place the regression assertions in scripts/__tests__ coverage.

Source: Path instructions

const validatePath = path.resolve(__dirname, "../../build/test/validate.umd.js");
// Add DOMPurify script
const domPurifyPath = path.resolve(
Expand Down
Loading