Overriding the timer doesn't really work... created Jasmine-specific monitoring for completion

[johnomalley]

The timeout_instrumenter solution is a clever idea but I don't think it's going to work. In my experience builds were only working because the timeout was hit and you specifically swallow the TimeoutException.

First of all, there were two showstopper bugs:

• In GenericInstrumentingBrowser you were checking __saga_timeouts.length but that property will be undefined as __saga_timeouts is a JS Object or hash - not an array.
• The code assumes that clearTimeout is invoked for every setTimeout. I'm pretty sure that's not the case in the real world (unless the JS engine invokes window.clearTimeout internally - in my testing it does not appear to do that) ... clearTimeout is only used to cancel a timer.

It may be possible to fix these bugs (I started to then decided to go a different way), but then the coverage will fail (actually it will just rely on the wait timeout of 5 minutes or whatever's configured) unless developers are very careful to avoid setInterval in tests. A single setInterval that isn't cleared will cause the coverage build to hang until it times out.

In my organization we have 10-20 projects and growing that use the same build process. We're trying to integrate coverage into that process. I guarantee you that some of our developers will not be so careful.

If you look at jasmine-maven-plugin you'll see that it relies on the jasmine reporter to indicate when it's done. That's what I've implemented here.

My first thought was to make the completion expression a configuration option (and I could certainly do that if you want), but it occurs to me that undoubtedly many if not most of your users are also jasmine users, given the fact that saga integrates well with Jasmine.

With this change the plugin works well on my AMD/Jasmine project. It also runs in less than 30 seconds.

Maybe you could add to the list of restrictions for PhantomJSDriver (or any driver other than HTMLUnit) that it only works with Jasmine, or it only works due to the timeout with a framework other than Jasmine. If you need to support another framework it should be easy to do so (see completion_monitor.js).

Thanks

[timurstrekalov]

To be honest, I do not know what the hell I was thinking when I was doing this. As you've correctly pointed out, relying on clearTimeout being called was, well, dumb.

I think your solution should be more than enough, since most people use Jasmine anyway 😄

[timurstrekalov]

just released with 1.5.3