Skip to content

Commit

Permalink
Fix running browser tests in CI
Browse files Browse the repository at this point in the history
Apparently mocha-headless doesn’t work with newer Chrome/Chromium
versions – it broke on my local system a while ago, and eventually also
in CI. I haven’t been able to find a working replacement (Karma looked
most promising, but seems to be married to RequireJS too tightly to
support modules as test files), and in the meantime Node released
experimental fetch() support – it’s not good enough to replace the axios
backend (no cookie support), but enough to run the browser tests. So for
now, we run the “browser” tests under Node as well, at least in CI and
when you run `npm t`; but also add a browser-test.html file that can be
loaded manually in a browser (using `python -m http.server` or
equivalent – it won’t work using the file:// protocol), and tweak
browser.test.js so that the Chai import works in Node and browser.

This requires bumping the test-full CI job to Node 18; I’m generally
happy to do that, since test-most (Node 12) still checks support for
older Node versions, but at some point Node removed the
--harmony-top-level-await option, so we need to tweak the test:readme
command to retry running Node without it. (Fortunately we don’t have to
repeat the sed command, when Node complains about the invalid option it
doesn’t consume any stdin.)

I hope to eventually replace this with proper testing in a browser
again, see #23.
  • Loading branch information
lucaswerkmeister committed Sep 18, 2022
1 parent 42d2771 commit 2a8449f
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 5 deletions.
3 changes: 3 additions & 0 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
"plugin:chai-friendly/recommended",
"plugin:compat/recommended"
],
"parserOptions": {
"sourceType": "module"
},
"rules": {
"comma-dangle": [ "error", "always-multiline" ],
"no-use-before-define": [ "error", "nofunc" ],
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ jobs:
- uses: actions/checkout@v2
- uses: actions/setup-node@v2
with:
node-version: 14
node-version: 18
- run: npm ci
- run: npm test
env:
Expand All @@ -29,7 +29,7 @@ jobs:
MEDIAWIKI_USERNAME: ${{ secrets.MEDIAWIKI_USERNAME }}
MEDIAWIKI_PASSWORD: ${{ secrets.MEDIAWIKI_PASSWORD }}
# don’t run test:lint in Node 12 – no point running it twice
# don’t run test:browser in Node 12 – mocha-headless needs ?? syntax
# don’t run test:browser in Node 12 – needs Node fetch() (see #23)
# don’t run test:readme in Node 12 – needs --harmony-top-level-await
test-package-lock:
runs-on: ubuntu-latest
Expand Down
22 changes: 22 additions & 0 deletions browser-test.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8" />
<title>m3api Browser Tests</title>
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<link rel="stylesheet" href="node_modules/mocha/mocha.css" />
</head>
<body>
<div id="mocha"></div>

<script src="node_modules/mocha/mocha.js"></script>
<script class="mocha-init">
mocha.setup('bdd');
mocha.checkLeaks();
</script>

<script type="module" src="test/integration/browser.test.js"></script>
<script type="module">mocha.run()</script>
</body>
</html>

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
"test:lint": "eslint .",
"test:unit": "mocha test/unit/",
"test:node": "mocha test/integration/node.test.js",
"test:browser": "mocha-headless test/integration/browser.test.js",
"test:readme": "sed -n '/```/,/```/ { /```$/q; /```/n; p; }' README.md | node --input-type=module --harmony-top-level-await"
"test:browser": "mocha test/integration/browser.test.js # #23",
"test:readme": "sed -n '/```/,/```/ { /```$/q; /```/n; p; }' README.md | { node --input-type=module --harmony-top-level-await || node --input-type=module; }"
},
"homepage": "https://github.com/lucaswerkmeister/m3api#readme",
"bugs": "https://github.com/lucaswerkmeister/m3api/issues",
Expand Down
8 changes: 8 additions & 0 deletions test/integration/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"parserOptions": {
"ecmaVersion": "latest"
},
"rules": {
"es/no-global-this": "off"
}
}
19 changes: 18 additions & 1 deletion test/integration/browser.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,24 @@
/* eslint-env mocha */

/*
* This test is supposed to run in a browser,
* but there’s currently no great way to do that automatically,
* so in CI and `npm t` it actually runs in Node.js,
* using its experimental fetch() support.
* It would be great to fix this – see #23.
* (You can still manually run it in a browser using browser-test.html.)
*/

import BrowserSession, { set } from '../../browser.js';
import '../../node_modules/chai/chai.js'; /* globals expect */
import '../../node_modules/chai/chai.js'; /* globals chai */

/* chai isn’t a proper module so we have to import it differently in Node */
let expect;
if ( 'chai' in globalThis ) {
expect = chai.expect;
} else {
expect = ( await import( '../../node_modules/chai/chai.js' ) ).default.expect;
}

describe( 'BrowserSession', function () {

Expand Down

0 comments on commit 2a8449f

Please sign in to comment.