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

moral of the story: quote your globs on the command-line #2355

Closed
sverraest opened this issue Jul 5, 2016 · 9 comments
Closed

moral of the story: quote your globs on the command-line #2355

sverraest opened this issue Jul 5, 2016 · 9 comments

Comments

@sverraest
Copy link

I'm using Mocha.js as part of a CI pipeline.
Recently the failing tests don't actually cause the npm test step to fail the build because it looks like Mocha.js doesn't exit in an failure state.

ERROR: {"error":{"message":"Cannot read property 'where' of undefined","stack":"TypeError: Cannot read property 'where' of undefined\n at Object.undeleted
info: Done.
npm info posttest api@0.0.0
npm info ok

This is quite annoying as it allows commits with failing tests to still be build in our CI pipeline.

@gurdiga
Copy link

gurdiga commented Jul 11, 2016

Hi @sverraest! 🙂

I’m trying to reproduce this and I seem not to be able to get there. 😳

Here is my setup:

    ~/tmp/mocha ɀ  mocha --version
2.5.3
    ~/tmp/mocha ɀ  cat test.js 
it('runs tests', function() {
  assert(false); // this will cause an error because there is no `assert` in the scope
});
    ~/tmp/mocha ɀ  grep "test" package.json 
    "test": "mocha test.js"
    ~/tmp/mocha ɀ  npm test

> mocha@1.0.0 test /Users/vladgurdiga/tmp/mocha
> mocha test.js



  1) runs tests

  0 passing (15ms)
  1 failing

  1)  runs tests:
     ReferenceError: assert is not defined
      at Context.<anonymous> (test.js:2:3)



npm ERR! Test failed.  See above for more details.
    ~/tmp/mocha ɀ  echo $?
1
    ~/tmp/mocha ɀ  

So it looks like a runtime error causes Mocha to fail. 🤔

I’m wondering if you could share a bit of your test code, just to get a bit more context around this issue and maybe be able to reproduce and find a solution. 🤓

@boneskull boneskull added unconfirmed status: waiting for author waiting on response from OP - more information needed labels Jul 11, 2016
@boneskull
Copy link
Contributor

@sverraest Can you provide more context for the failure?

@unkillbob
Copy link

I've also been observing this recently, originally I thought the issue was with protractor or gulp-protractor but I notice the issue only seems to be when the bail: true option is configured for mocha.

@Munter
Copy link
Contributor

Munter commented Oct 4, 2016

@unkillbob can you provide a failing test and a command line that triggers this?

@unkillbob
Copy link

@Munter after making that observation I tried to do exactly that but couldn't reproduce it. Either its intermittent or it could be something else in the stack that's to blame - if I ever do manage to reproduce it reliably will be sure to post it here.

@ibratoev
Copy link

ibratoev commented Nov 2, 2016

I hit the same issue and spent some time to extract a reproducible case. It is a bit more complex so I put it in a quick repo: https://github.com/ibratoev/mocha-repro .
@boneskull, does this confirms the issue?

ibratoev added a commit to ibratoev/mocha-repro that referenced this issue Nov 2, 2016
@boneskull boneskull added confirmed and removed status: waiting for author waiting on response from OP - more information needed unconfirmed labels Nov 2, 2016
@boneskull boneskull changed the title Failed tests not ending with error state allowing CI to continue Undefined behavior when executing files which are not tests Nov 2, 2016
@boneskull boneskull added type: bug a defect, confirmed by a maintainer status: accepting prs Mocha can use your help with this one! labels Nov 2, 2016
@boneskull
Copy link
Contributor

Well, the problem is a little more complicated than this, but I'm not sure why. running npm test vs executing mocha directly results in different behavior.

@boneskull boneskull removed type: bug a defect, confirmed by a maintainer confirmed status: accepting prs Mocha can use your help with this one! labels Nov 2, 2016
@boneskull
Copy link
Contributor

My previous comment (which I deleted) went down the wrong path. After more tinkering, this has to do with the shell.

Basically, tests/**/*.js means something different in say, bash (by default)--than in glob, e.g.:

$ ls tests/**/*.js
tests/tests/bar.js

vs

$ node -e "console.log(require('glob').sync('tests/**/*.js'))"
[ 'tests/tests/bar.js', 'tests/tests/tests2/foo.js' ]

So, yeah. There's nothing Mocha can really do about this. You have to change your shell's behavior, or provide a string value for the glob. In Bash 4.x, you can:

$ shopt -s globstar
$ ls tests/**/*.js
tests/tests/bar.js      tests/tests/tests2/foo.js

I don't know what the equivalent is in Bash 3.x or zsh (though I'd like to know!). Note that MacOS X ships with 3.x; use something like Homebrew to upgrade.

Otherwise, this will work (and is the strategy Mocha uses in its Makefile for cross-env compatibility):

$ mocha 'tests/**/*.js'


  Test
    1) Fake test


  0 passing (8ms)
  1 failing

@boneskull boneskull changed the title Undefined behavior when executing files which are not tests moral of the story: quote your globs on the command-line Nov 3, 2016
@ibratoev
Copy link

ibratoev commented Nov 3, 2016

Thanks for the insight @boneskull. My scenario really has nothing to do with mocha but more with how npm works. Some related links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants