Modernize test infrastructure and linting toolchain - #1003
Conversation
4cd8bd2 to
b1286e4
Compare
Drops the geckodriver functional test (test/functional/badging.test.js) as webextensions-geckodriver is abandoned. This test should be re-added using Playwright. Fixes #1002
…ntion Adds two Playwright tests covering the remaining patternDetection paths: one verifies login, share, and passive-share elements each get the correct badge class; another confirms the fbc-has-badge guard prevents an element from being badged more than once. Fixes #1006
web-ext resolves the firefox alias via mdfind bundle ID lookup, which returns both Release and Beta since they share org.mozilla.firefox — alphabetical ordering causes Beta to win. Adds a gitignored local config and example template so developers can pin a specific binary. Fixes #1013
b1286e4 to
2acace3
Compare
@eslint/js 10's recommended config enables no-useless-assignment, which flags a dead assignment in addHeaderWithBackArrow: the return value of addHeader() was immediately overwritten. The call is kept for its side effect of appending the header to the fragment; only the discarded binding is removed.
990ff60 to
147fc67
Compare
| let el = addHeader(fragment); | ||
| el = document.createElement("button"); | ||
| addHeader(fragment); | ||
| const el = document.createElement("button"); |
There was a problem hiding this comment.
Note that this is the only change to the actual extension code.
It was necessary to get ESLlint to v10 without any rule overrides
There was a problem hiding this comment.
question (non-blocking): the PR description seems out-dated. it says the functional badging test is dropped and should be restored with playwright. This looks like the badging tests in playwright?
question (blocking): the old tests drove real Firefox thru geckodriver and loaded the extension at a moz-extension:// URL. this new test serves fixtures and stubs window.browser so would this test actually catch something like a failure to inject the content script, or a bug where injecting the content script breaks something on the page? that seems like the kind of bug that could really break users' experience.
it seems like we're replacing 2 abandoned forks of projects that drove a real browser with this new mock. might be the right thing to do, but it also means future green check-marks mean less about real browser behavior than old ones did. if we're still sending FBC releases thru full QA cycles that's fine. but let's not merge this until we're confident in the answer here.
| "test-functional-dev": "cross-env NODE_ENV=development npm run test-functional", | ||
| "coverage": "nyc --reporter=html --reporter=text mocha --reporter=progress test/setup.js test/features/*.test.js --timeout 60000", | ||
| "test": "npm run test:lint && npm run test:unit", | ||
| "test:lint": "eslint src test/features", |
There was a problem hiding this comment.
question (non-blocking): this no longer lints the test/functional/ directory - is that intentional?
| const onCompletedListeners = []; | ||
| const onErrorOccurredListeners = []; | ||
|
|
||
| function makeListener() { |
There was a problem hiding this comment.
question (non-blocking): is this ever actually called?
2 participants
Our linting setup was running ESLint 4 (released 2018) and our test suite depended on
webextensions-jsdomandwebextensions-geckodriver, two abandoned forks. This replaces both with modern alternatives.ESLint is upgraded to v10 with the new flat config format. The test runner moves from mocha to vitest, backed by a small custom browser mock that we own and can maintain. All 34 existing tests pass unchanged. Other outdated packages (sinon, chai, web-ext, etc.) are brought up to date and
npm auditis clean.Note - The one functional test (badging, via geckodriver) is dropped for now and should be re-added with Playwright.
Additionally, I added a setup file for CircleCI which is running the new test suite successfully. This check will run against all future PRs too.
Testing
npm auditwarnings.