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

karma-mocha: revisit browser test framework solution #2487

Closed
boneskull opened this issue Sep 19, 2016 · 3 comments
Closed

karma-mocha: revisit browser test framework solution #2487

boneskull opened this issue Sep 19, 2016 · 3 comments
Labels
status: accepting prs Mocha can use your help with this one!

Comments

@boneskull
Copy link
Member

The current state of things is that we use karma-no-mocha, which is a fork of karma-mocha with the peer dep of mocha@* removed.

Different versions of npm do different things with the peer dependency:

  • npm v1/v2 would attempt to install mocha@* to satisfy the peer dep, and karma-mocha would end up using it for testing--which is exactly what we don't want to happen.
  • npm v3 warns about the peer dep instead, which is better, but still a warning, and bad DX.

I question the suitability of mocha as a peer dependency of karma-mocha. If anything, the peer dependency seems like it should be karma--it's a Karma plugin, not a Mocha plugin. It doesn't seem like it should be a hard dependency either. I'll bring this up over yonder.

Anyhow.

Open to other possible solutions. Maintaining a fork sucks.


The following solution is a workaround, which would be superior if done correctly.

Execute a preinstall script that effectively does:

  • mkdir -p /path/to/mocha/node_modules
  • ln -sf /path/to/mocha /path/to/mocha/node_modules/mocha

The concerns I have about this approach are:

  • Portability is a question mark; we'd have to implement the preinstall script using the vanilla fs module, and take care not to clobber an actual directory or linked package (though I don't know why this would ever happen).
  • This preinstall script should only be run if the prepublish script (which doesn't exist, but if it did) would be run after npm install. So how do we know if prepublish should be run? This is how.

Similar to the above, we could simply defer installation of karma-mocha until make test-browser is run. We could then use something like shelljs to help, and wouldn't need to worry about lifecycle scripts. I'm not sure there's a precedent for this type of behavior, though; it definitely smells funny.


If it's possible to tell npm 2.x and/or npm 3.x to ignore peer dependencies via an .npmrc or something, that'd be great as well. @othiym23 do you know if this is possible?

@ScottFreeCode
Copy link
Contributor

ScottFreeCode commented Sep 24, 2016

Isn't there an environment variable we can set to add the mocha project directory to NPM's/Node's search paths as an alternative using a symlink in node_modules? I'd have to dig up the name, though -- NODE_ENV or NODE_PATH or something like that... And, I'm not sure whether it could be set in such a way as to get NPM to recognise Mocha's already "installed" as opposed to merely working at the point of looking up Mocha for use (I think we might already be setting it for the latter, but what I don't know is whether you can set it for the former).

[ETA:] Yup, I should have read the linked Karma-Mocha issue first; it's already brought up there that we're using the variable to handle requiring our own Mocha instance. So I guess I should have just asked: is there any way to get that variable to affect NPM checking whether Mocha's installed too?

@boneskull
Copy link
Member Author

AFAIK there's no way in NPM to disable automatic installation of peer dependencies and/or warnings.

@boneskull
Copy link
Member Author

This has been addressed

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!
Projects
None yet
Development

No branches or pull requests

2 participants