From 448e97636c5505360681f2bfdc84dbe33391a43d Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Sun, 26 May 2024 16:28:43 +0100 Subject: [PATCH] Core: Deprecate unset timeout for tests taking longer than 3 seconds Up until now, QUnit has not defined or enforced any default timeout. This means if your test timed out or got stuck, this would be silent and could (depending on how you run/integrate your tests) be hard to find the cause of, and might take a long time to even detect as it would be subject to a higher-level timeout (e.g. some kind of maximum process time or CI limit). In QUnit 3.0, the default timeout will change from undefined (effectively Infinity) to 3 seconds. This only effects you if you * do not have `QUnit.config.testTimeout` set, * and, have one or more tests exceeding 3 seconds, * and, are not calling `assert.timeout()` inside this test. Starting in QUnit 2.21, a deprecation warning will be logged, at most once per test run, if you have no timeout defined, and a test takes longer than 3 seconds. You can prepare yourself for QUnit 3 when this happens by calling `assert.timeout()` inside those tests, or by setting `QUnit.config.testTimeout` globally with a higher timeout as-needed. --- docs/api/config/testTimeout.md | 44 ++++++++++++++++++- src/assert.js | 2 +- src/core.js | 2 +- src/core/config.js | 1 + src/test.js | 12 +++++ test/cli/cli-main.js | 1 + .../fixtures/config-testTimeout-deprecated.js | 17 +++++++ .../config-testTimeout-deprecated.tap.txt | 14 ++++++ test/cli/fixtures/hanging-test.tap.txt | 3 +- 9 files changed, 91 insertions(+), 5 deletions(-) create mode 100644 test/cli/fixtures/config-testTimeout-deprecated.js create mode 100644 test/cli/fixtures/config-testTimeout-deprecated.tap.txt diff --git a/docs/api/config/testTimeout.md b/docs/api/config/testTimeout.md index 1ed253406..494433889 100644 --- a/docs/api/config/testTimeout.md +++ b/docs/api/config/testTimeout.md @@ -9,7 +9,7 @@ redirect_from: version_added: "1.0.0" --- -Set a global default timeout in milliseconds after which a test will fail. This helps to detect async tests that are broken, and prevents tests from running indefinitely. +Set a global default timeout in milliseconds after which a test will fail. This helps to detect async tests that are broken, and prevents a test run from hanging indefinitely. @@ -22,4 +22,44 @@ Set a global default timeout in milliseconds after which a test will fail. This
-This can be overridden on a per-test basis via [assert.timeout()](../assert/timeout.md). If you don't have per-test overrides, it is recommended to set this to a relatively high value (e.g. `30000` for 30 seconds) to avoid intermittent test failures from unrelated delays one might in a browser or CI service. +This can be overridden on a per-test basis via [assert.timeout()](../assert/timeout.md). + +It is recommended to keep the global default at `3000` (3 seconds) or higher, to avoid intermittent test failures from unrelated delays that may periodically occur inside a browser or CI service. + +### Deprecated: No timeout set + +Starting in QUnit 2.21, a deprecation warning will be logged if a test takes longer than 3 seconds, when there is no timeout set. + +``` +Test {name} took longer than 3000ms, but no timeout was set. +``` + +You can prepare yourself for QUnit 3 when this happens, by either calling `assert.timeout()` inside those tests, or by setting `QUnit.config.testTimeout` once globally with a higher timeout. + +Depending on your test runner of choice, there may be more convenient ways to set configuration: + +* Set `qunit_config_testtimeout` via [preconfig](../config/index.md) as environment variables (for Node.js), or as global variables for HTML/browser environments (before QUnit is loaded). +* Set `testTimeout` via [karma-qunit](https://github.com/karma-runner/karma-qunit/#readme): + ``` + config.set({ + frameworks: ['qunit'], + plugins: ['karma-qunit'], + client: { + qunit: { + testTimeout: 5000 + } + } + }); + ``` + +* and, are not using a test runner that sets its own timeout (e.g. [grunt-contrib-qunit](https://github.com/gruntjs/grunt-contrib-qunit)), + +### Introducing a timeout + +Prior to QUnit 3, there has not been a default timeout. This meant that a test could time out or get stuck for many seconds or minutes before diagnostic details are presented (e.g. after a CI job reaches the maximum run time). + +QUnit 3.0 will change the default timeout from undefined (Infinity) to 3 seconds. + +### Changelog + +| UNRELEASED | Announce change of default from undefined to `3000`, with a deprecation warning. diff --git a/src/assert.js b/src/assert.js index 25c541eb3..94cc8e7f7 100644 --- a/src/assert.js +++ b/src/assert.js @@ -81,7 +81,7 @@ class Assert { // Alias of pushResult. push (result, actual, expected, message, negative) { Logger.warn('assert.push is deprecated and will be removed in QUnit 3.0.' + - ' Please use assert.pushResult instead (https://qunitjs.com/api/assert/pushResult).'); + ' Please use assert.pushResult instead. https://qunitjs.com/api/assert/pushResult'); const currentAssert = this instanceof Assert ? this : config.current.assert; return currentAssert.pushResult({ diff --git a/src/core.js b/src/core.js index 3c9404122..8bf783b05 100644 --- a/src/core.js +++ b/src/core.js @@ -124,7 +124,7 @@ extend(QUnit, { load: function () { Logger.warn('QUnit.load is deprecated and will be removed in QUnit 3.0.' + - ' Refer to .'); + ' https://qunitjs.com/api/QUnit/load/'); QUnit.autostart(); }, diff --git a/src/core/config.js b/src/core/config.js index 0387d69bd..4e34207a2 100644 --- a/src/core/config.js +++ b/src/core/config.js @@ -127,6 +127,7 @@ const config = { // started: 0, // Internal state + _deprecated_timeout_shown: false, blocking: true, callbacks: {}, modules: [], diff --git a/src/test.js b/src/test.js index 880cd7fde..9cb68bf14 100644 --- a/src/test.js +++ b/src/test.js @@ -759,6 +759,18 @@ Test.prototype = { config.timeoutHandler(timeoutDuration), timeoutDuration ); + } else { + clearTimeout(config.timeout); + config.timeout = setTimeout( + function () { + config.timeout = null; + if (!config._deprecated_timeout_shown) { + config._deprecated_timeout_shown = true; + Logger.warn(`Test "${test.testName}" took longer than 3000ms, but no timeout was set. Set QUnit.config.testTimeout or call assert.timeout() to avoid a timeout in QUnit 3. https://qunitjs.com/api/config/testTimeout/`); + } + }, + 3000 + ); } } diff --git a/test/cli/cli-main.js b/test/cli/cli-main.js index 314d58384..4e919002f 100644 --- a/test/cli/cli-main.js +++ b/test/cli/cli-main.js @@ -16,6 +16,7 @@ QUnit.module('CLI Main', () => { // and only await/assert the already-started command. concurrentMapKeys(readFixtures(FIXTURES_DIR), 0, (runFixture) => runFixture()), async (assert, fixture) => { + assert.timeout(10000); const result = await fixture; assert.equal(result.snapshot, result.expected, result.name); } diff --git a/test/cli/fixtures/config-testTimeout-deprecated.js b/test/cli/fixtures/config-testTimeout-deprecated.js new file mode 100644 index 000000000..8f7147bd9 --- /dev/null +++ b/test/cli/fixtures/config-testTimeout-deprecated.js @@ -0,0 +1,17 @@ +QUnit.test('fast async', function (assert) { + var done = assert.async(); + assert.true(true); + setTimeout(done, 7); +}); + +QUnit.test('slow async 1', function (assert) { + var done = assert.async(); + assert.true(true); + setTimeout(done, 3500); +}); + +QUnit.test('slow async 2', function (assert) { + var done = assert.async(); + assert.true(true); + setTimeout(done, 3500); +}); diff --git a/test/cli/fixtures/config-testTimeout-deprecated.tap.txt b/test/cli/fixtures/config-testTimeout-deprecated.tap.txt new file mode 100644 index 000000000..73c3189f3 --- /dev/null +++ b/test/cli/fixtures/config-testTimeout-deprecated.tap.txt @@ -0,0 +1,14 @@ +# command: ["qunit", "config-testTimeout-deprecated.js"] + +TAP version 13 +ok 1 fast async +ok 2 slow async 1 +ok 3 slow async 2 +1..3 +# pass 3 +# skip 0 +# todo 0 +# fail 0 + +# stderr +Test "slow async 1" took longer than 3000ms, but no timeout was set. Set QUnit.config.testTimeout or call assert.timeout() to avoid a timeout in QUnit 3. https://qunitjs.com/api/config/testTimeout/ diff --git a/test/cli/fixtures/hanging-test.tap.txt b/test/cli/fixtures/hanging-test.tap.txt index 449758e4a..a364f2c87 100644 --- a/test/cli/fixtures/hanging-test.tap.txt +++ b/test/cli/fixtures/hanging-test.tap.txt @@ -4,7 +4,8 @@ TAP version 13 # stderr +Test "hanging" took longer than 3000ms, but no timeout was set. Set QUnit.config.testTimeout or call assert.timeout() to avoid a timeout in QUnit 3. https://qunitjs.com/api/config/testTimeout/ Error: Process exited before tests finished running Last test to run (hanging) has an async hold. Ensure all assert.async() callbacks are invoked and Promises resolve. You should also set a standard timeout via QUnit.config.testTimeout. -# exit code: 1 \ No newline at end of file +# exit code: 1