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

Specify Karma version compatibility #13

Closed
pirelenito opened this issue Sep 14, 2016 · 7 comments
Closed

Specify Karma version compatibility #13

pirelenito opened this issue Sep 14, 2016 · 7 comments

Comments

@pirelenito
Copy link

Hi there! Awesome project!

I am having issues making the launcher run and it looks like it is because my Karma version was older than the required one.

I looked into this Karma PR and it seems that karma-electron requires Karma at least 1.1.0, is that so?

If that is correct, should we add it as a peerDependency? I can prepare a PR.

@twolfson
Copy link
Owner

We do require at least karma@1.1.0. I figured we wouldn't need to document/introduce that because people who are installing karma-electron will typically only be using this plugin which means their karma install would be fresh.

In addition to that, I would prefer to avoid peerDependencies as they typically cause more headaches than solve any:

npm/npm#5080

How about we add something to the README as a compromise?

@pirelenito
Copy link
Author

On npm 3 this shouldn't be a problem anymore, given it doesn't install peerDependencies automatically. Its purpose is simply to warn any consumer of the package that you need to install extra dependencies for it to work.

If you still are not comfortable in adding it as a peerDependencies I would do as you suggest and add a notice in the README.

Thanks again!

@pirelenito
Copy link
Author

Either way you decide to go, I can prepare a PR.

@twolfson
Copy link
Owner

Ah, that's a good solution for npm@3. Unfortunately, a lot of users (myself included) are still using npm@2 (my reason is this blocker issue hasn't bee resolved to my knowledge -- npm/npm#10119)

I think the README is the best solution. We should probably also list the version of Electron we have tested against.

I will take care of the README update -- I'm typically picky about what's said/where so I don't want to waste any of your time with back and forth. I will take care of that by the end of the weekend.

For my own reference, how were you using karma before karma-electron? (e.g. using PhantomJS but with a bundler)

@pirelenito
Copy link
Author

Awesome, thanks!

I still use Karma with PhantomJS for my regular web projects. You can check how I set it up in Sagui a tool to build frontend projects.

My use-case with your library is to test an actual Electron app.

Would you suggest using it for regular web applications as well? And why?

@twolfson
Copy link
Owner

This has been patched and released in 4.1.1

@twolfson
Copy link
Owner

Ah, alright. Thanks for the context =)

As a heads up, this is best suited for only testing the renderer portion of an electron app (i.e. testing menus should be done via Electron and Selenium)

I wouldn't suggest using this for regular web apps. It's typically best to have the testing environment be as close to production as possible. This means the ideal scenario is using an actual browser (e.g. Chrome, Firefox), although both electron and PhantomJS suffice.

One scenario where it would annoyingly fail would be accidentally relying on something Node.js based but something is different in the browser polyfill (e.g. Buffer). One scenario where it would be nice is visual regression testing directly in the test suite (maybe using it as a secondary plugin, instead of exclusively).

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

2 participants