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

Public API does not provide hooks for server started, all browsers captured, etc #1037

Closed
lazd opened this issue Apr 18, 2014 · 23 comments
Closed

Comments

@lazd
Copy link

lazd commented Apr 18, 2014

Problem

The public API does not provide enough insight into the state of the server.

Why?

Because server tasks that are dependent on its state can be triggered programmatically by outside entities, it is necessary for said entities to have knowledge of the server's state.

For instance, imagine if you wanted to do the following, programmatically:

  1. Start a Karma server (server.start())
  2. When all browsers are connected, run tests (runner.run())
  3. Trigger additional runs at arbitrary times (such as when files change) (runner.run())

Currently, this is not possible. Yes, this is exactly what autoWatch does, but runs cannot being triggered programatically. Yes, singleRun will run tests when all browsers are connected, but it will kill the server and exit when tests are done running, which makes the third step impossible.

Workaround

In order to work around this, implementors are forced watch process.stdout for messages to infer what's going on inside of Karma, which is incredibly brittle and hackish.

Solution

I tried digging to find a way to get access to the event emitter that triggers some of these events, but it seems it's not currently possible via the public API.

If server.start returned and object with a reference the globalEmitter object, we'd be halfway there.

@pkozlowski-opensource
Copy link
Member

@lazd is your request mainly in the scope of usage with gulp? If so, could we move this discussion to https://github.com/karma-runner/gulp-karma instead? At the moment I'm doing a number of changes in karma so it exists properly and we can use its public API with callbacks.

@lazd
Copy link
Author

lazd commented Apr 19, 2014

It's related, but it's an issue with Karma's public API so I filed here.

We need more than exit callbacks -- we need events for browser capture etc. Any thoughts on how that can be exposed as part of the public API?

@pkozlowski-opensource
Copy link
Member

I think that we can extend the public API given a well-defined use-case. At the moment I'm focused on making Karma exit properly, but would be keen on looking into other practical scenarios after that.

What is your practical need for those additional hooks?

@vojtajina vojtajina added this to the Backlog milestone Apr 23, 2014
@vojtajina
Copy link
Contributor

I think this is a valid feature request... Maybe we could allow registering events on globalEmitter...

@lazd
Copy link
Author

lazd commented Apr 24, 2014

@pkozlowski-opensource, the practical need is to know when Karma is in certain states so we can trigger test runs (as described above).

@vojtajina, registering events on globalEmitter might be sufficient, but it may cause the expected public API to break when when Karma's internals change (removed events, event names changed). Furthermore, the events emitted by globalEmitter are underscore_separated, whereas configuration file keys are camelCase, and it would make sense for the public events to be camelCase as well. As such, it may be better to expose a specific subset of events triggered on globalEmitter on the objects returned by start() and run() under different names.

Without knowing if all of these are possible, I imagine the events could be something like this:

runner

  • runStart - Test run started
  • runComplete - Test run completed
  • runFailure - Test run completed with failing tests
  • runSuccess - Test run completed with all tests passing

server

  • browserRegister - Browser registered, but not ready
  • browserReady - Browser ready to run tests
  • browserRunStart - Browser started test run
  • browserRunComplete - Browser finished test run
  • browsersReady - All browsers are ready
  • browsersChange - Browser removed or added

I'm not sure what other events are available or useful, but server.browsersReady is the one that's blocking my use case.

@pkozlowski-opensource
Copy link
Member

@lazd OK, I was going over all the workflows I'm using currently and I wonder what is the exact workflow you are trying to address. While I can see value in providing all those events, it seems to me that the real issue is to be able to run tests when a server (started with server.start) is ready. But it seems to me that it should be responsibility of runner.run to wait for the server to be ready (or, in other words, server should get instructions from runner and execute tests when ready). The other element that would be missing is the server.stop command or similar.

if I'm not mistaken, you are after the workflow where there is an external watch (grunt, gulp etc.) that watches files and should:

  • start Karma server when a watch is starting: server.start
  • re-run tests: runner.run - this should be aware of the server state
  • stop Karma when a watch is stopped: server.stop - this is missing today

What is your exact use-case? Would the above work for you?

@lazd
Copy link
Author

lazd commented May 7, 2014

@pkozlowski-opensource the use case you described above would work nicely for what I'm trying to do, which is basically something like this: https://github.com/lazd/gulp-karma-test/blob/master/gulpfile.js#L17-L26

The above use case would be possible if the runner is responsible for waiting until the server is ready, but it might be of benefit to still expose the server's state via events for more advanced tools. I could imagine the events described above being useful for VM providers (like Sauce and Browserling) that want to connect browsers and know when they're ready/crashed/done/etc.

@pkozlowski-opensource
Copy link
Member

@lazd ok, we are on the same page then! I can see that you are doing some work on the gulp + karma integration and wonder if we could join forces to have a no-nonsense integration of the 2. This is why we've started this repo: https://github.com/karma-runner/gulp-karma as atm I'm not sure how much added value we could get from a dedicated plugin.

@lazd
Copy link
Author

lazd commented May 7, 2014

@pkozlowski-opensource, yes let's work together on this!

From the start, I knew gulp-karma wasn't the right way to do this, but we simply didn't have the hooks in Karma we needed to get the job done in a sane way. Furthermore, it's not a valid gulp plugin as it doesn't work with Vinyl file objects. We simply need a guide that shows people how to use gulp with Karma, or possibly a thin layer (like the helper branch of gulp-karma) that makes it easy.

@pkozlowski-opensource
Copy link
Member

@lazd yup, I've noticed that this is not the best candidate for a plugin, this is why I wanted to have this repo with a integration scenario. At the moment my biggest worry is that Karma doesn't exit cleanly, there are 2 PRs waiting to be merged:

as soon as those are in we can look into the server-waiting-thing and see what utils / helpers people might need and where is the best place to implement them.

@lazd
Copy link
Author

lazd commented May 7, 2014

@pkozlowski-opensource, those were my biggest concerns as well, resulting in the spawn of an external process in the both the Grunt and gulp plugins.

I had to do some very dirty stuff to know when to start running tests for the watch, hence the filing of this issue. This comment elaborates on all of the above.

@pkozlowski-opensource
Copy link
Member

Yes, I know, it took me some time to tracks down all those non-exiting-properly issues but I believe that I've nailed all of them.

@abierbaum
Copy link

@pkozlowski-opensource @lazd Any updates on how this is going? I think there are several of us waiting for this to get through and working or find out what the recommended path forward is for now.

@pkozlowski-opensource
Copy link
Member

@abierbaum I must say that for now I was using the gulp+karma integration scenario demonstrated here https://github.com/karma-runner/gulp-karma and I must say that I'm very happy with this so far. As of now there is an issue on Windows (karma-runner/gulp-karma#5) but it is well understood and we've got a PR with a fix waiting to be merged. So my immediate goal is to make the https://github.com/karma-runner/gulp-karma workflow working for everyone, including people on Windows.

Now, it is a bit hard for me to say what are the best next steps as I must say that I'm pretty happy with the integration scenario indicated above. I guess you guys got different use cases / favourite workflows so to get things moving I would ideally need the description of your perfect working scenario with an ideal syntax you would like to see.

@abierbaum
Copy link

@pkozlowski-opensource Thanks for the pointer to this path. That is what I needed. I will give it a try and see how well it works for our case. We have browserify (with watchify) and gulp-watch for sass files. My biggest concern is getting everything working for the "watching" case where I want karma, browserify, and everything else to update automatically on any change. If I run into any issues I will try to create a test case I can share publically to show any problems.

@lazd
Copy link
Author

lazd commented Jul 2, 2014

@abierbaum, the biggest blocker I had for a similar workflow was knowing when Karma was ready to run the first round of tests. I needed to do something like:

  1. Start Karma, launch and capture browsers
  2. Then, run tests
  3. Then, start watching
  4. When files change, run tests

Currently, there is no way to know:

a. When Karma is ready to run tests (browsers have been captured)
b. When Karma is done running tests

Without this information, it's possible that tests are not ran because no browsers have been captured or because a change happened in the middle of the first run.

In order to extract that information, I had to do some awful, dirty things in a branch that I'm not comfortable releasing. However, the API resulting from these awful hacks does get the job done, check it out.

Now that @pkozlowski-opensource has fixed the exit errors, the dirty hack of spawning another process for Karma is not needed, but until this issue is closed, we'll still be spying on Karma's stdout to figure out what it's doing.

@pkozlowski-opensource
Copy link
Member

@lazd I must say that I didn't put much more effort into this topic (apart from the obvious timeout fixes) simply because I'm quite happy / comfortable with my current workflow. The thing is that I never had to to what you are describing, that is, manually start karma, wait for browsers to connect and then run tests. I guess the reason for this is that I'm working as follows:

  • working in the TDD mode while developing (no single run) - in this case Karma will start the server, start browsers etc. Even if in this case one run is missed it is not a big deal since I'm going to change the code in few next seconds / minutes and test will be re-run.
  • having one-time builds (locally or CI). In this case I'm simply using the single-run option to karma and it takes care of starting the server, lunching browsers, waiting for them to capture and closing everything when done. In this case time taken to start the server + lunch browser(s) is not an issue since this is a one-off build.

So, I would be very keen on understanding your workflow where you need to "manually" start Karma, lunch browsers etc. The only use-case I can see is to integrate with external watchers. While I agree that it would be great if we could plug any external watching mechanism, the current one built-in in Karma works for me, so I'm not too pressed to dig into this.

What are your real-life workflow scenarios that you are trying to tackle (apart from integrating with external watchers?).

@lazd
Copy link
Author

lazd commented Jul 2, 2014

@pkozlowski-opensource, I understand your workflow and why you wouldn't need this.

Running tests immediately

I like the tests to run immediately before I start watching. If the tests fail, this lets me know that things were broken before I started changing code and prevents me from scratching my head as to why a change in an unrelated file caused tests to fail when, actually, the code was broken in the first place.

Yes, we could do a single run with Karma, then when it exits, start watching with Karma, but we're launching/closing browsers all the time and we wouldn't have to do that if we had a browsersReady event.

Integration with watchers

If you're building your app, say with browserify, you're watching for source file changes, then building a bundle, and then you'd like to run tests on the bundle.

Yes, we can let Karma watch the bundle file (output) and get away with it, but I'd rather use one watcher.

I can't quite recall what the problem was, but I had trouble getting the server to recognize a new list of files between runs, so if tests were added, it wouldn't accept them. I would have to dig back into it to give any kind of useful troubleshooting information, but something wasn't right. If this is still an issue, it's a separate one for sure, but it serves to highlight the problems we're having unless we let Karma do the watching.

@pkozlowski-opensource
Copy link
Member

@lazd thnx for the detailed info, I'm happy that we can focus on concrete use-cases (disclaimer: I'm not saying that Karma shouldn't provide hooks that you are proposing, just trying to understand the situation better!).

Now, for the running tests immediately - I'm guess we might have a different setup, but when I do gulp tdd in this project: https://github.com/karma-runner/gulp-karma all the tests are run after Karma starts and a browser is captured. Do you see missed runs / no initial run on your end?

Then, the use-case with Browserify is more interesting. I'm not using it myself but it is very likely that I will need to dive into its usage soon. But I'm using JS written the CommonJS-style at the daily basis and this is why I've spend more time improving https://github.com/karma-runner/karma-commonjs. What I'm trying to say here is that I would probably even try to avoid running browserify bundling while running tests. I do acknowledge that the https://github.com/karma-runner/karma-commonjs plugin doesn't solve everything (for example, it doesn't have any fallback for the native modules provided by browserify). Would this plugin work for you?

Once again, I think your requests are valid, I'm just trying to better understand what people are looking for, practically speaking.

@abierbaum
Copy link

@lazd Have you successfully used gulp, karma, and browserify together? I am currently having trouble figuring out how to get karma and browserify to work correctly in this context. (ie. how to have tests that karma is running use require() to pull in modules that are under test and similarly to evaluate requires within the hierarchy of test files)

@lazd
Copy link
Author

lazd commented Jul 2, 2014

@abierbaum, I haven't given it a fair shot, I've been busy on other projects. I would suggest generating a browserify bundle with your tests and including that in Karma along side your application bundle. Given the fact that browserify transforms could change things, it's best to serve Karma a ready-made bundle.

@pkozlowski-opensource
Copy link
Member

@abierbaum your watching issue is probably the same as reported here: #1113 and has nothing to do with the issue discussed here. Would be awesome if you could move your comment over to #1113 in order to not diverge from the original topic.

@abierbaum
Copy link

@pkozlowski-opensource Will do. I will just delete it from this thread for now. Thanks.

nikaspran pushed a commit to nikaspran/karma that referenced this issue Jul 4, 2015
Return the EventEmitter instance so that it's possible to hook
into various lifecycle events. Add a "browsers_ready" event so
that it's possible to know when the karma server has finished
starting up.

Enables and partially resolves karma-runner#1037
@dignifiedquire dignifiedquire modified the milestones: v1.0, Backlog Jul 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants