mantoni / mocaccino.js

Mocha test runner as a Browserify plugin
MIT License
22 stars 12 forks source link

Error when running mochify with v1.6 of mocaccino #13

Closed chrisdwheatley closed 8 years ago

chrisdwheatley commented 8 years ago

I'm running my unit tests through mochify which depends upon mocaccino (^1.5).

Our tests seem to have started failing in our CI environment over the weekend. I inspected the version of mocaccino our CI box was running compared to my local dev environment (where the tests were still passing) and noticed the CI box was 1.6 whereas locally I'm using 1.5.2.

The error I'm seeing is:

/foo/node_modules/mochify/node_modules/resolve/lib/sync.js:33
    throw new Error("Cannot find module '" + x + "' from '" + y + "'");
          ^
Error: Cannot find module 'spec' from '/foo/node_modules/mochify/node_modules/mocaccino/lib'
    at Function.module.exports [as sync] (/foo/node_modules/mochify/node_modules/resolve/lib/sync.js:33:11)
    at module.exports (/foo/node_modules/mochify/node_modules/mocaccino/lib/mocaccino.js:44:34)
    at Browserify.plugin (/foo/node_modules/mochify/node_modules/browserify/index.js:349:9)
    at module.exports (/foo/node_modules/mochify/lib/mochify.js:131:5)
    at Object.<anonymous> (/foo/web/node_modules/mochify/bin/cmd.js:27:1)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Function.Module.runMain (module.js:497:10)

I verified the issue by installing version 1.6 of mocaccino locally where I saw the error. I then removed 1.6 and manually install 1.5.2 (npm install mocaccino@1.5.2) from within the mochify/node_modules directory and our tests began to work again.

I took a quick look at the code but nothing stood out to me as the cause of the issue.

In the meantime I've forked mochify and amended the dependency to use the tilde rather than caret for the dependency. I'm about to create a PR to pull that back upstream for others who may be seeing the same issue.

mantoni commented 8 years ago

This is odd. The change that was made is to check if the given reporter is a known mocha reporter. If it's unknown, mocaccino will try to resolve it as a custom reporter. For some reason, spec is considered an unknown reporter. I can't reproduce on my end. Can you provide more details on the mocha version installed and how you use it? Are you using the API or the command line with some switches? Which OS and node version are you using?

mkxml commented 8 years ago

Hi @swirlycheetah. I don't know what version of mocha you are using. Try specifying mocaccino to use Spec, with big S, like mocaccino -R Spec. In my version of mocha it works both ways but maybe your version of mocha has not registered spec on the reporter list.

If that's the case maybe we just need to change the validation to be case-insensitive or something like that.

chrisdwheatley commented 8 years ago

Thanks for the responses. I'll check everything you've mentioned in the morning when I have access to the codebase.

It's a recently inherited codebase which I'm still getting to grips with so apologies for being unable to answer any of your questions right now.

chrisdwheatley commented 8 years ago

Here's some more details to hopefully help:

node.js 0.10.25 npm 2.14.1 mochify 2.13.0 mocha 2.2.5 OS: OS X 10.9.5 and a linux VM which runs CI

Using the command line version, here's the full command I'm using,

mochify --colors --reporter spec ./src/test/unit/client/specs/*.js && NODE_ENV=test node ./scripts/run-unit-tests.js

The script (./scripts/run-unit-tests.js) looks to be firing up mocha and then running through the tests, happy to put that into a gist if it'd be useful.

I also tried the above with --reporter Spec and received the following error;

"Spec" reporter blew up with error:
Error: failed to require "Spec"
      at null:null
Error: invalid reporter "Spec"
      at null:null

Just to be clear, I'm not using mocaccino directly at all, I'm only dependent upon it via mochify.

mantoni commented 8 years ago

Thanks @swirlycheetah. @mkautzmann had the right idea. Mocha 2.2.5 does not export the lower case reporter names. The aliases where introduced in Mocha 2.3.0.

We should either support Mocha < 2.3.0 or bump it in the mocaccino dependencies.

Would anyone be able to send a pull request for this?

mkxml commented 8 years ago

I vote for bumping Mocha's version, sent a PR about that.

mantoni commented 8 years ago

@mkautzmann Thanks!

mkxml commented 8 years ago

Hey @swirlycheetah. It looks that @mantoni already updated everything, can you test it again and give us feedback if it's fine now?