busterjs / buster

Abandoned - A powerful suite of automated test tools for JavaScript.
http://docs.busterjs.org
Other
448 stars 37 forks source link

buster-coverage + buster-amd #415

Closed gxxcastillo closed 9 years ago

gxxcastillo commented 10 years ago

When I run buster-test I get the "all slaves timed out" message after a long pause. This problem, however, does not happen if I disable the buster-coverage extension.

Looking around, I found a few old threads that mentioned the similar problems and I was wondering if there's a updated state of affairs?

https://groups.google.com/forum/#!msg/busterjs-dev/S0E64D4wBqQ/rV870UoSXHcJ https://github.com/ebi/buster-coverage/issues/10 http://irclogger.com/.buster.js/2013-07-18

dwittner commented 9 years ago

For me buster-coverage + buster-amd work together very well. Could you provide example code to reproduce the issue?

gxxcastillo commented 9 years ago

Will do, hopefully by the end of the week, thanks.

dwittner commented 9 years ago

@uglymunky, do you have any news?

gxxcastillo commented 9 years ago

damn, not yet. Will look into this today.

gxxcastillo commented 9 years ago

Just tested this out in a staw-man type setup and it worked fine.

That said, enabling buster-coverage continues to break things on my real setup. Gonna dig a little deeper and see if I can narrow things down a bit more.

gxxcastillo commented 9 years ago

Here's what I got:

Enabling buster-coverage results in an Uncaught ReferenceError: Invalid left-hand side in assignment

What triggers it seems to be any occurrences of an assignment operator within a complex logical expression, like so:

    var val;
    if (val || (val = 'someVal')) {
        val = 'result';
    }

This error happens wether the library is loaded from a "source" file or from a "test" file. Also, it only happens when buster-coverage is enabled.

Unfortunately, many 3rd party libraries are full of this kind of stuff (including jQuery). It seems like when there are enough of these it messes things up and eventually results in a timeout...however, I didn't dig much deeper.

If you think it would be helpful, I could put an example up on github.

dwittner commented 9 years ago

@uglymunky, yes, please put an example up on github. I can't reproduce the issue, not even with the code block you provided. Where do you see the Uncaught ReferenceError, in the browser console?

gxxcastillo commented 9 years ago

Here ya go:

https://github.com/uglymunky/coverage-amd

You should be able to just clone it and immediately run the tests.

Yeah, the error is in the browser console but it also shows up in the terminal.

dwittner commented 9 years ago

@uglymunky, with the example from https://github.com/uglymunky/coverage-amd, i can reproduce the issue:

Uncaught exception: ./dep/trouble.js:20 Uncaught ReferenceError: Invalid left-hand side in assignment
Uncaught exception in Unknown context:

  undefined
dwittner commented 9 years ago

@uglymunky, the cause of the problem seems to be in burrito, the library buster-coverage is based on. It just removes the parentheses around val = 'someVal':

define(function(){"use strict";var val;if(val||val="someVal"){val="result"}return{test:val}});

The consequence is, that the code isn't valid any longer. Unfortunately the burrito project seems to be dead. Further buster-coverage isn't maintained by the Buster.JS project team and even worse, it isn't maintained anymore at all. I'm thinking about, to take over the project to the Buster.JS project. But the more i think about it, the more i am against it. We would have to refactor it, so that it no longer depends on burrito. In my opinion, that's not worth it, especially because there is another coverage extension for Buster.JS, buster-istanbul. It is based on istanbul, which seems to be maintained a lot better, than burrito. I haven't tried it out yet, but maybe you can give it a try.

gxxcastillo commented 9 years ago

Sweet, I'll check it out, thanks!

dwittner commented 9 years ago

@uglymunky, let me know, if buster-istanbul is useful for you.

abhishekbh commented 8 years ago

Just as an update here, the same problem seems to occur with buster-coverage. I switched to buster-istanbul and we seem to be good.

gxxcastillo commented 8 years ago

ah, shoot, just realized I never responded. Indeed, buster-istanbul has worked for me as well.