Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 Bug: --timeout 0 does not disable timeouts #4834

Open
4 tasks done
bdcarr opened this issue Feb 24, 2022 · 3 comments
Open
4 tasks done

馃悰 Bug: --timeout 0 does not disable timeouts #4834

bdcarr opened this issue Feb 24, 2022 · 3 comments
Labels
status: accepting prs Mocha can use your help with this one! type: bug a defect, confirmed by a maintainer

Comments

@bdcarr
Copy link

bdcarr commented Feb 24, 2022

Prerequisites

  • Checked that your issue hasn't already been filed by cross-referencing issues with the faq label
  • Checked next-gen ES issues and syntax problems by using the same environment and/or transpiler configuration without Mocha to ensure it isn't just a feature that actually isn't supported in the environment in question or a bug in your code.
  • 'Smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, your usage of Mocha, or Mocha itself
  • Ensured that there is no discrepancy between the locally and globally installed versions of Mocha. You can find them with: node_modules/.bin/mocha --version(Local) and mocha --version(Global). We recommend that you not install Mocha globally.

Description

I ran a mocha test using IntelliJ's debugger, which automatically sets --timeout 0. Left it paused for several minutes while looking at something, and when I resumed it, it timed out.

Steps to Reproduce

  1. Create a test script containing a single describe() and a single it() (you can do more, this is just a simple example)
  2. Inside the describe(), put this.timeout(T) (I haven't tried with the timeout() call inside the it(), not sure if behaviour changes)
  3. Ensure somehow that the it() will take longer than T to run: set a breakpoint, or add code that will take longer than that
  4. Run the test with --timeout 0

Working example

  1. Check out https://github.com/bdcarr/mocha-timeout-bug/tree/main
  2. cd newversion
  3. npm install
  4. npm run test

There's also an oldversion folder which contains the same test script, but the package.json is set to use mocha@7.2.1, which is the last version where this issue doesn't occur. I checked with v8.0.0 and the issue occurs.

Expected behavior: No matter how long the test takes, it will not time out if run with --timeout 0

Actual behavior: The test times out after T milliseconds

2022-02-25 10-35-05

Reproduces how often: 100%

Versions

  • The output of mocha --version and node_modules/.bin/mocha --version: No global install. Issue reproduced on v8.0.0 and v9.2.1. Issue does not occur on v7.2.1.
  • The output of node --version: v14.9.0
  • Your operating system
    • name and version: MacOS 12.2.1
    • architecture (32 or 64-bit): 64-bit
  • Your shell (e.g., bash, zsh, PowerShell, cmd): zsh v5.8
  • Any third-party Mocha-related modules (and their versions): n/a
  • Any code transpiler (e.g., TypeScript, CoffeeScript, Babel) being used (and its version): n/a, straight NodeJS with no imports
@juergba juergba added type: bug a defect, confirmed by a maintainer and removed unconfirmed-bug labels Mar 1, 2022
@juergba
Copy link
Member

juergba commented Mar 1, 2022

@bdcarr thank you for this detailed bug report.
I can reproduce different behaviour between Mocha v7.2.0 and v9.2.1.

@juergba
Copy link
Member

juergba commented Mar 10, 2022

#4260 has caused this issue. It allows the overwriting of timeout 0 which is bad in the debug mode.

I don't know yet how to fix this, evtl.:

  • a "global" timeout of 0 (set via CLI --timeout 0) should never be overwritten
  • or we have to re-introduce an internal state as disableTimeout

@bdcarr
Copy link
Author

bdcarr commented Apr 11, 2022

Workaround to prevent overriding a 0 timeout:

function setMochaTimeout(timeout, context) {
  const getRootSuite = (context) => {
    if (context.parent) return getRootSuite(context.parent)
    return context
  }
  
  // if this is called from inside an it(), we have to look at the `test` property instead of directly on the context
  if (!context.hasOwnProperty('_timeout')) context = context.test
  
  const rootTimeout = getRootSuite(context)._timeout
  
  if (rootTimeout !== 0) context.timeout(timeout)
}

...

it('some test', function() {
  setMochaTimeout(5000, this)
  ...
})

edit: updated the workaround function. The original one didn't work when called inside an it(), and didn't allow you to set the timeout to 0 and then change it to something else. Now it'll check the actual root timeout value, which seems to always reflect what it was when the script was started, so you can even set the root suite's timeout to 0 and still get the desired result.

Example test:
2022-06-17 11-47-56

Normal execution:
2022-06-17 11-48-18

With --timeout 0:
2022-06-17 11-49-08

@JoshuaKGoldberg JoshuaKGoldberg changed the title --timeout 0 does not disable timeouts since v8.0.0 馃悰 Bug: --timeout 0 does not disable timeouts Dec 27, 2023
@JoshuaKGoldberg JoshuaKGoldberg added the status: accepting prs Mocha can use your help with this one! label Dec 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: accepting prs Mocha can use your help with this one! type: bug a defect, confirmed by a maintainer
Projects
None yet
Development

No branches or pull requests

3 participants