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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Bug: --delay and grep not working reliably #3007

Open
4 tasks done
plasticrake opened this issue Sep 18, 2017 · 2 comments
Open
4 tasks done

🐛 Bug: --delay and grep not working reliably #3007

plasticrake opened this issue Sep 18, 2017 · 2 comments
Labels
semver-major implementation requires increase of "major" version number; "breaking changes" status: accepting prs Mocha can use your help with this one! type: bug a defect, confirmed by a maintainer

Comments

@plasticrake
Copy link

Prerequisites

  • Checked that your issue isn't already filed by cross referencing issues with the common mistake 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 node_modules/.bin/mocha --version(Local) and mocha --version(Global). We recommend avoiding the use of globally installed Mocha.

Description

I am running mocha with --delay and calling run(). Grep works unreliable with my tests unless I specify the exact file.

Steps to Reproduce

I created a gist to replicate: https://gist.github.com/plasticrake/71e6a69de1c9bf8d8234ab597cde6a77

mocha --delay

  Sync Mocha
    Will it grep3?
      ✓ it grepped3!

  Async Mocha
    Will it grep1?
      ✓ it grepped1!

  Async Mocha
    Will it grep2?
      ✓ it grepped2!

  3 passing (5ms)

grep1 works

mocha --delay -f grep1

  Async Mocha
    Will it grep1?
      ✓ it grepped1!

  1 passing (4ms)

grep3 works

mocha --delay -f grep3

  Sync Mocha
    Will it grep3?
      ✓ it grepped3!

  1 passing (4ms)

grep2 does not work

mocha --delay -f grep2


  0 passing (1ms)

However grep2 works if I specify the file:

mocha --delay test/mi2.js -f grep2


  Async Mocha
    Will it grep2?
      ✓ it grepped2!


  1 passing (5ms)

Expected behavior: grep to work consistantly

Actual behavior: grep fail to match SOME tests, but not all

Reproduces how often: always for certain files

Versions

mocha 3.5.3
node v8.5.0

Additional Information

@markowsiak
Copy link
Contributor

Hi @plasticrake

Thank you for submitting this issue. I've confirmed from your gist that the problem is reproducible, and further to that thatmocha --delay -f grep2 works in the absence of the other two test files. There is certainly some inconsistency here.

Thanks again for reporting 👍

@markowsiak markowsiak added type: bug a defect, confirmed by a maintainer confirmed labels Sep 21, 2017
@ScottFreeCode
Copy link
Contributor

I've dug into this a bit and determined a few things:

  • run should only be called once total, not once per file.
    • The documentation needs to be updated to make this clear.
  • If you comment out the run calls and add setTimeout(run, 5); in just one test file then the grepping becomes consistent as it should be; effectively, there's some kind of race condition when it's possible for one test file's asynchronous action to call run at about the same time another test file's asynchronous action defines its tests.

That's obviously a contrived example, and just as the real test scenario is probably not using setTimeout the real solution is going to involve having a central point where all the asynchronous actions can be waited for and then run called once after they're all ready. It's not going to be pretty but that's the immediate fix. (Also don't forget that --delay is only needed if the number of tests or their names are asynchronously determined; for all other cases regular asynchronous techniques with before or beforeEach will do, even if they have to be outside any describe so they apply to the test run as a whole.)

We can consider changing this behavior in a major version bump: instead of run being supposed to be called once, require run to be called once per file. We'd have to track the test files that get loaded and then either ensure that each one gets run called in it once or, if we can't tell which file run is being called from, ensure at any rate that there is the same number of calls to run as there are test files, before actually running the testsuite.

@ScottFreeCode ScottFreeCode added type: feature enhancement proposal status: waiting for author waiting on response from OP - more information needed status: needs review a maintainer should (re-)review this pull request type: question support question and removed type: bug a defect, confirmed by a maintainer confirmed labels Sep 22, 2017
@JoshuaKGoldberg JoshuaKGoldberg changed the title --delay and grep not working reliably 🐛 Bug: --delay and grep not working reliably Dec 28, 2023
@JoshuaKGoldberg JoshuaKGoldberg added type: bug a defect, confirmed by a maintainer semver-major implementation requires increase of "major" version number; "breaking changes" and removed type: feature enhancement proposal status: waiting for author waiting on response from OP - more information needed type: question support question status: needs review a maintainer should (re-)review this pull request labels Dec 28, 2023
@JoshuaKGoldberg JoshuaKGoldberg added the status: accepting prs Mocha can use your help with this one! label Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-major implementation requires increase of "major" version number; "breaking changes" 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

4 participants