Add GitHub Actions CI and make the puppeteer suite run headless with no local server - #771
Merged
joedolson merged 6 commits intoAug 25, 2026
Merged
Conversation
- ableplayer-base.js: the descriptionsAudible else-if duplicated the first condition (no-dupe-else-if), so the documented singular spelling data-description-audible was never honored. Read the singular attribute (options.descriptionAudible ?? data.descriptionAudible) in the second branch, matching the existing comment's intent. - dialog.js: import AblePlayer for the AblePlayer.getActiveDOMElement() call (no-undef). - dragdrop.js: remove unused thisObj locals in handleWindowButtonClick and handleMenuChoice (no-unused-vars); document the intentionally-empty Space/Enter branch (no-empty). - preference.js: remove the write-only $thisLabel variable and a dead $thisField assignment (no-unused-vars, no-useless-assignment). npm run lint: 0 problems. jsdom test suites: 55/55 pass. Build artifacts intentionally omitted per contributing.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 4, 2026
Closed
Per review: the singular `data-description-audible` alternate code path was added in the feature's original commit as protection against misspelling the attribute, but was never documented and never the standard form. Eliminate the alternate condition rather than keep a lint-clean version of it; `data-descriptions-audible` (plural) is the one supported spelling.
The repository ships Jest and ESLint configs but has no CI, so regressions surface only when someone runs the tools locally. This workflow runs three jobs on every push and pull request: - ESLint over scripts/ (npm run lint) - the jsdom Jest project (55 tests; the puppeteer project needs a localhost:8000 demo server and a headed browser, so it is excluded for now) - a Grunt + Rollup build with an artifact existence check, without committing build output (per contributing.md) Node 22 with npm cache; read-only GITHUB_TOKEN permissions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
validate.test.cjs previously required a manually started server on localhost:8000 and a headed browser (headless: false), so npm test always failed 26 tests out of the box and the suite could not run in CI. - jest-puppeteer.config.cjs: headless by default; HEADFUL=1 restores a visible browser for debugging; pass --no-sandbox in CI containers. - validate.test.cjs: navigate to an intercepted http://ableplayer.test/ origin fulfilled from memory instead of localhost:8000. A real http(s) origin is still required because isProtocolSafe() resolves relative URLs against window.location.origin (opaque on about:blank), but no server process is needed. - ci.yml: new test-browser job runs the puppeteer project (after npm run build, which produces build/test/validate.umd.js). npm test now passes 81/81 locally with zero setup. Build artifacts intentionally omitted per contributing.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- The jsdom project's webvtt.test.cjs loads build/test/webvtt.umd.js, so without a build step the job exercised the committed bundle instead of the source under review. Build first, matching the puppeteer job. - Cancel superseded runs on the same ref so a force-push to an open pull request does not leave stale jobs queued. - Give every job an explicit timeout; the default is six hours, which a hung headless page load would otherwise occupy. - jest-puppeteer: compare HEADFUL explicitly so HEADFUL=0 stays headless instead of launching a visible browser (any non-empty string is truthy).
blogcastAI
force-pushed
the
upstream/headless-tests
branch
from
August 10, 2026 03:58
19d4f0b to
adb378a
Compare
…esting docs - Only the puppeteer job launches a browser, but puppeteer's postinstall downloads a browser during npm ci in every job, and setup-node's npm cache does not cover that directory. Set PUPPETEER_SKIP_DOWNLOAD on the lint, jsdom and build jobs. - contributing.md described adjusting the test runner's target URL and a default of http://localhost:8000. The suite now intercepts its own navigation and needs no server, so document that, and mention HEADFUL=1 for watching the browser while debugging.
blogcastAI
added a commit
to blogcastAI/ableplayer
that referenced
this pull request
Aug 10, 2026
…#773 review fixes into develop # Conflicts: # .github/workflows/ci.yml # e2e/a11y.spec.js # e2e/keyboard.spec.js # e2e/serve.mjs # jest-puppeteer.config.cjs # playwright.config.js
joedolson
approved these changes
Aug 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #770 (includes its lint commit so CI starts green — happy to rebase once that merges).
What
Two changes that make the existing test suite protective:
1. GitHub Actions CI (
.github/workflows/ci.yml) — the repo ships ESLint + Jest configs but no CI. Four jobs on every push/PR: ESLint, Jest jsdom (55 tests), Jest puppeteer headless (26 tests), and a Grunt + Rollup build with artifact existence checks (build output never committed, per contributing.md). Node 22, npm cache,GITHUB_TOKENrestricted tocontents: read.2. Headless, serverless puppeteer suite —
npm testcurrently fails 26/81 out of the box:validate.test.cjsexpects a manually-started server onlocalhost:8000, andjest-puppeteer.config.cjssetsheadless: false(needs a display).jest-puppeteer.config.cjs: headless by default,HEADFUL=1to watch while debugging,--no-sandboxunder CI.validate.test.cjs: request interception replaces the server — the page navigates tohttp://ableplayer.test/fulfilled from memory. A real http(s) origin is still needed becauseisProtocolSafe()resolves relative URLs againstwindow.location.origin(opaque onabout:blank), but no server process.Verification
npm test: 81/81 pass locally with zero setup (was 55/81 + manual server)Why
contributing.md calls test-suite expansion a high priority — CI plus a suite that runs unattended is the prerequisite that makes new tests protective rather than advisory. We run Able Player v5 in production and plan further contributions (YouTube issue cluster #736/#670/#606 next).
🤖 Generated with Claude Code