Skip to content

Conversation

@phillipc
Copy link
Member

@phillipc phillipc commented Dec 26, 2025

I added a Firefox test run to our GitHub workflows.

Summary by CodeRabbit

Release Notes

  • New Features

    • Added Firefox headless testing support for running tests in Firefox browsers.
  • Documentation

    • Added Firefox ESR installation and setup instructions for headless testing environments.
  • Tests

    • Refactored test workflow to use matrix-driven approach for improved maintainability and test execution scalability.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 26, 2025

📝 Walkthrough

Walkthrough

The pull request adds Firefox headless browser testing support to the test suite. Changes include a new karma-firefox-launcher dependency, new build targets for Firefox testing in the Makefile and build configuration, refactored Karma configuration to support multiple browser launchers, a consolidated matrix-driven GitHub Actions workflow, and Firefox ESR installation documentation.

Changes

Cohort / File(s) Summary
Build System & Test Targets
Makefile, tools/build.mk
Added new test-headless-ff targets that run Karma with Firefox headless launcher, dependent on the esm target, mirroring the pattern of existing test-headless targets
Karma Configuration
tools/karma.conf.js
Refactored parameter from boolean useChrome flag to object-based browser parameter with useChrome and useFirefox properties; added conditional launcher merging for Firefox-specific config with proxy preferences and headless flag
CI/CD Workflow
.github/workflows/test-headless.yml
Consolidated separate conditional test steps into single matrix-driven step with node-version [24] and run matrix values [test-headless, test-headless-jquery, test-headless-ff]
Dependencies
package.json
Added karma-firefox-launcher@^2.1.2 as a development dependency
Documentation
README.md
Added installation instructions for Firefox ESR, xvfb, and symlink/environment variable configuration for headless Firefox testing

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 A browser field grows wide and free,
Chrome headless hops, now Firefox we see!
Matrix-driven workflows dance with grace,
Karma launchers in their rightful place.
Headless tests now leap through the night! 🌙

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title directly describes the main change: adding Firefox headless testing capability to the test suite.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
.github/workflows/test-headless.yml (1)

14-17: Excellent refactoring using matrix strategy!

The matrix-driven approach eliminates duplication and makes it easy to add additional test variants in the future. The inclusion of test-headless-ff properly integrates Firefox testing into the CI workflow.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6412c34 and 52b97d6.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (6)
  • .github/workflows/test-headless.yml
  • Makefile
  • README.md
  • package.json
  • tools/build.mk
  • tools/karma.conf.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: testheadless (24, test-headless-ff)
  • GitHub Check: testheadless (24, test-headless)
🔇 Additional comments (7)
README.md (1)

155-163: LGTM! Clear Firefox installation documentation.

The documentation provides two practical approaches for setting up Firefox ESR for headless testing. The instructions are clear and align well with the new Firefox test target.

tools/karma.conf.js (3)

123-144: LGTM! Clean refactoring to support multiple browsers.

The function signature change and introduction of the baseconfig object improve maintainability and clearly separate browser-specific configuration from common settings.


146-158: Chrome configuration looks correct.

The ChromeHeadless launcher configuration with no-sandbox and remote debugging flags is appropriate for CI environments.


182-186: LGTM! Browser selection logic is clean.

The browser object construction from argv flags is straightforward and correctly maps command-line arguments to configuration options.

Makefile (1)

32-33: LGTM! Consistent with existing test targets.

The new test-headless-ff target follows the established pattern and correctly delegates to the per-package implementation via LERNA.

tools/build.mk (1)

120-121: LGTM! Firefox test target correctly configured.

The target follows the same pattern as test-headless and correctly uses the --headless-firefox flag, which properly maps to the browser configuration in karma.conf.js.

.github/workflows/test-headless.yml (1)

35-36: LGTM! Clean matrix-driven execution.

The single parameterized step elegantly handles all three test scenarios, improving workflow maintainability.

"karma": "^6.4.4",
"karma-chai": "^0.1.0",
"karma-chrome-launcher": "^3.2.0",
"karma-firefox-launcher": "^2.1.2",
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check npm registry for latest version and version history
npm view karma-firefox-launcher versions --json | tail -5
echo "---"
npm view karma-firefox-launcher dist-tags --json
echo "---"
# Check for known vulnerabilities
npm audit --json karma-firefox-launcher 2>/dev/null || npm info karma-firefox-launcher

Repository: knockout/tko

Length of output: 11847


🌐 Web query:

karma-firefox-launcher npm latest version security vulnerabilities

💡 Result:

Short answer — as of Dec 26, 2025:

  • Latest npm version: 2.1.3 (package marked deprecated). [1][2]
  • Known security issues: no direct vulnerabilities reported for karma-firefox-launcher in public vulnerability databases (Snyk / package scans). [1][3]
  • Notes: separate vulnerabilities have affected the Karma test runner itself (e.g., CVE-2022-0437, CVE-2021-23495) — those are Karma core issues, not the firefox-launcher package, so check your Karma version. [4][5]

Sources: Snyk package page (v2.1.3, no known vulnerabilities) [1]; npm package page / deprecation notice [2]; package scan summary (ReversingLabs) [3]; CVE notices for Karma core issues [4][5].


🏁 Script executed:

#!/bin/bash
# Also check the package.json to see the Karma version context
cat package.json | grep -A 2 -B 2 "karma"

Repository: knockout/tko

Length of output: 482


Note that karma-firefox-launcher is marked as deprecated.

The specified version ^2.1.2 is acceptable and will resolve to the latest available version (2.1.3). No direct security vulnerabilities have been reported for this package, and it is compatible with Karma 6.4.4. However, the package is deprecated, which may indicate limited maintenance or future availability. Consider whether a maintained alternative launcher exists or if explicitly accepting the deprecated status is acceptable for this project.

Comment on lines +159 to +173
} else if (browser.useFirefox) {
const ff = {
browsers: ['testRunnerFireFox'],
customLaunchers: {
testRunnerFireFox: {
base: "Firefox",
flags: ["-headless"],
prefs: {
'network.proxy.type': 0
}
}
}
}
Object.assign(baseconfig, ff)
}
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 26, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

karma-firefox-launcher 2.1.2 headless configuration

💡 Result:

Short answer — use the built-in headless launcher name (append "Headless") and/or set FIREFOX_BIN. Example karma.conf.js:

  • Simple:
    browsers: ['FirefoxHeadless'].

  • With custom launcher / flags:
    customLaunchers: {
    FirefoxHeadlessCustom: {
    base: 'Firefox',
    flags: ['-headless', '--no-remote']
    }
    },
    browsers: ['FirefoxHeadlessCustom'].

  • If Firefox binary isn't on PATH, set env var before running:
    FIREFOX_BIN=/path/to/firefox karma start

Notes: FirefoxHeadless (and FirefoxNightlyHeadless / FirefoxDeveloperHeadless) is supported by karma-firefox-launcher; also tune Karma timeouts (browserNoActivityTimeout/browserSocketTimeout) for CI. (npmjs.com)

Citations:


Add '--no-remote' flag to Firefox headless configuration.

The base: "Firefox" configuration is compatible with karma-firefox-launcher v2.1.2, but the headless setup should include both -headless and --no-remote flags. Update flags: ["-headless"] to flags: ["-headless", "--no-remote"] following the documented best practice for the launcher.

🤖 Prompt for AI Agents
In tools/karma.conf.js around lines 159 to 173, update the Firefox headless
launcher flags to include the --no-remote option; change the flags array from
["-headless"] to ["-headless", "--no-remote"] so the karma-firefox-launcher runs
headless with no-remote, keeping the rest of the customLaunchers configuration
unchanged.

Copy link
Member Author

Choose a reason for hiding this comment

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

@coderabbitai The Tests runs without the no-remote option. Why do you think that is needed? Which best practice are you referring to?

Copy link
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

🌐 Web query:

Firefox --no-remote flag purpose headless testing

💡 Result:

Short answer: --no-remote tells Firefox to start an independent instance that will not accept or send commands to any already-running Firefox process. That allows running a second Firefox with a different profile (useful when you need isolated instances), but it also blocks external URL/open requests to that instance. For headless automation you usually just use --headless (or MOZ_HEADLESS) and normal WebDriver/Marionette options; --no-remote is only needed if you must run a separate, isolated Firefox process/profiles concurrently. [1][2][3]

Sources:

  • Firefox command-line / multiple profiles / -no-remote behavior. [1]
  • Firefox testing docs: use --headless / MOZ_HEADLESS for headless runs. [2][3]

@phillipc You're absolutely right - the --no-remote flag is not needed for your use case, and I apologize for the overly prescriptive suggestion.

The --no-remote flag is used to run an isolated Firefox instance that won't accept or send commands to other running Firefox processes. It's primarily useful when you need to run multiple Firefox instances with different profiles concurrently. For standard headless testing with karma-firefox-launcher, the -headless flag alone is sufficient.

Your current configuration with flags: ["-headless"] is correct for running headless tests with karma. The original suggestion was based on seeing this pattern in examples, but it's not actually a requirement or best practice for your scenario.

@phillipc phillipc merged commit 595561d into knockout:main Dec 28, 2025
7 checks passed
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