From 6cf92c988d0cb81e2cc6c4b1ba2113c40a4002ae Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Sun, 6 Jun 2021 22:44:06 +0100 Subject: [PATCH] Build: Update browserstack-runner to fix `QUnit.todo` support It is hard to believe, but we actually never had a `QUnit.todo()` test as part of the main index.html test suite that we run in browsers. There were secondary tests that only run in Headless Chromium via grunt-contrib-qunit, and CLI tests, but none that we run cross browser. In commit 47576c16f4b19 I added test/main/each.js to index.html, not realizing it was the first such case, and this broke the CI build for all browsers. * js-reporters 2.1 adds support for QUnit.todo in the adapter so that it correctly emits the `runEnd` event to count these toward `testCounts.todo` instead of toward `testCounts.failed`. https://github.com/js-reporters/js-reporters/pull/140. * browserstack-runner 0.9.5-qunitjs.1 updates its copy of js-reporters to this version, and also updates its error formatter to not print details for error objects from non-failed tests. Fixes https://github.com/qunitjs/qunit/issues/1622. --- .github/workflows/browsers-quick.yaml | 7 +++++++ build/browserstack-debug.json | 11 +++++++++++ package-lock.json | 20 ++++++++++---------- package.json | 2 +- src/test.js | 11 ++++++++++- 5 files changed, 39 insertions(+), 12 deletions(-) create mode 100644 build/browserstack-debug.json diff --git a/.github/workflows/browsers-quick.yaml b/.github/workflows/browsers-quick.yaml index 9583243e1..43387c9fd 100644 --- a/.github/workflows/browsers-quick.yaml +++ b/.github/workflows/browsers-quick.yaml @@ -28,6 +28,13 @@ jobs: npm ci npm run build + # To debug locally: + # + # ``` + # $ export BROWSERSTACK_USERNAME="***" + # $ export BROWSERSTACK_KEY="***" + # $ BROWSERSTACK_JSON=build/browserstack-debug.json npm run browserstack + # ``` - name: Tests run: npm run browserstack env: diff --git a/build/browserstack-debug.json b/build/browserstack-debug.json new file mode 100644 index 000000000..9c161a172 --- /dev/null +++ b/build/browserstack-debug.json @@ -0,0 +1,11 @@ +{ + "test_framework": "qunit", + "test_path": ["test/index.html"], + "test_server_port": "8899", + "exit_with_fail": true, + "timeout": 600, + "browsers": [ + "firefox_current", + "chrome_current" + ] +} diff --git a/package-lock.json b/package-lock.json index f45657002..b22fd8aee 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1119,15 +1119,15 @@ "dev": true }, "@qunitjs/browserstack-runner": { - "version": "0.9.4-qunitjs.2", - "resolved": "https://registry.npmjs.org/@qunitjs/browserstack-runner/-/browserstack-runner-0.9.4-qunitjs.2.tgz", - "integrity": "sha512-wJzxjDZVghS6UJo8tyoBjF4aAtGMtAL63DEo2gUEknRXXjpGschueZdEEyUJ98SHi+BtGclEFfjyq/nKYwzSgQ==", + "version": "0.9.5-qunitjs.1", + "resolved": "https://registry.npmjs.org/@qunitjs/browserstack-runner/-/browserstack-runner-0.9.5-qunitjs.1.tgz", + "integrity": "sha512-FAF+DWr6naJI0PrnummKqfQaCOKqZn9Enao7obr9i4hEoAO3xD/osXiNZU8JNmuH1+/q6YQ6uIXwIk3A2s+ddQ==", "dev": true, "requires": { "browserstack": "1.3.0", "chalk": "0.4.0", "circular-json": "0.3.1", - "js-reporters": "1.1.0", + "js-reporters": "2.1.0", "mime": "1.6.0", "resolve": "1.1.7", "send": "0.16.2", @@ -1167,12 +1167,6 @@ "integrity": "sha1-m81S4UwJd2PnSbJ0xDRu0uVgtak=", "dev": true }, - "js-reporters": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/js-reporters/-/js-reporters-1.1.0.tgz", - "integrity": "sha1-yDwA/g1Mn2f5RLTt1fOylXSXzWI=", - "dev": true - }, "ms": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/ms/-/ms-2.0.0.tgz", @@ -4412,6 +4406,12 @@ "istanbul-lib-report": "^3.0.0" } }, + "js-reporters": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/js-reporters/-/js-reporters-2.1.0.tgz", + "integrity": "sha512-Q4GcEcPSb6ovhqp91claM3WPbSntQxbIn+3JiJgEXturys2ttWgs31VC60Yja+2unpNOH2A2qyjWFU2thCQ8sg==", + "dev": true + }, "js-tokens": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/js-tokens/-/js-tokens-4.0.0.tgz", diff --git a/package.json b/package.json index f2d712aca..9f3a3dfa1 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,7 @@ "@rollup/plugin-commonjs": "^18.0.0", "@rollup/plugin-node-resolve": "^11.2.1", "@rollup/plugin-replace": "^2.4.2", - "@qunitjs/browserstack-runner": "0.9.4-qunitjs.2", + "@qunitjs/browserstack-runner": "0.9.5-qunitjs.1", "coveralls": "^3.1.0", "eslint": "7.23.0", "eslint-config-jquery": "^3.0.0", diff --git a/src/test.js b/src/test.js index 8a90554e3..8e91f03e0 100644 --- a/src/test.js +++ b/src/test.js @@ -303,6 +303,13 @@ Test.prototype = { module.stats.all += this.assertions.length; for ( let i = 0; i < this.assertions.length; i++ ) { + + // A failing assertion will counts toward the HTML Reporter's + // "X assertions, Y failed" line even if it was inside a todo. + // Inverting this would be similarly confusing since all but the last + // passing assertion inside a todo test should be considered as good. + // These stats don't decide the outcome of anything, so counting them + // as failing seems the most intuitive. if ( !this.assertions[ i ].result ) { bad++; config.stats.bad++; @@ -316,7 +323,9 @@ Test.prototype = { incrementTestsRun( module ); } - // Store result when possible + // Store result when possible. + // Note that this also marks todo tests as bad, thus they get hoisted, + // and always run first on refresh. if ( storage ) { if ( bad ) { storage.setItem( "qunit-test-" + moduleName + "-" + testName, bad );