Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Top level errors behavior #157

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
2 participants

In CI mode, error is printed and testing bails out immediatelly. In Dev mode, error is printed in the error pane but testing continues.

Fixed some test issues and onerror handler.

Top level errors behavior. In CI mode, error is printed and testing
bails out immediatelly. In Dev mode, error is printed in the error pane
but testing continues.

Fixed some test issues and onerror handler.
Collaborator

airportyh commented Jan 18, 2013

Thanks! Will take a look later today.

Hi, any news?

Collaborator

airportyh commented Jan 23, 2013

Sorry about the delay. I've been slammed. Will try to make some time today.

Collaborator

airportyh commented Jan 24, 2013

I am not comfortable making a top-level-error cause a bail on the test suite. I think this should be an option, since different frameworks would handle this differently. Thoughts?

I do this in CI mode only where the process is quit afterwards. Top level errors might actually influence further testing. Other thing is to produce sane TAP output. The tap producer is a third party product and quite simple. Would not be the output confusing saying all ok first then the top level error message will be printed?

jk

On Jan 24, 2013, at 6:33 AM, Toby Ho notifications@github.com wrote:

I am not comfortable making a top-level-error cause a bail on the test suite. I think this should be an option, since different frameworks would handle this differently.


Reply to this email directly or view it on GitHub.

Collaborator

airportyh commented Jan 24, 2013

I think I am leaning towards defaulting to top-level-error failing the test suite like what you have, if people disagree, then we give them an option.

Ok let's do that.

jk

On Jan 24, 2013, at 7:14 PM, Toby Ho notifications@github.com wrote:

I think I am leaning towards defaulting to top-level-error failing the test suite like what you have, if people disagree, then we give them an option.


Reply to this email directly or view it on GitHub.

Collaborator

airportyh commented Jan 28, 2013

I finally got a chance to actually test out this PR, sorry for the delay. One problem I've discovered is that this will only work if the top-level-error occurs before the test suite has finished running - but I don't think there's anyway to work around this issues. So for example if in the code being tested I throw a

setTimeout(function(){
    throw new Error();
});

in there, then the top-level-error generated here won't necessarily be recorded by Testem - if the test suite runs to completion without any asynchronicity.

Right, it will not be caught. But as you wrote, there is no workaround for this. However, IMHO test suite or the test adapter shall make sure all the code (even async) is finished before reporting back to the runner that all testing is done.

Collaborator

airportyh commented Jan 28, 2013

But there is no way to make such a guarantee - you don't get direct access to the event queue in the browser.

Well, in the app one can use async.js mechanisms to wait for the async code. Of course, setTimeout and friends will be slipped. Anyway, as you said, there is no workaround for this.

Collaborator

airportyh commented Jan 30, 2013

For Dev mode I am not sure I like the way the error is represented in your pr: I think I like the old way better, which the top level error message at the bottom panel in red, the text in the tab should probably be in red though.

Just curious, when you encountered this problem, what kind of top-level-error was it? Was it like a syntax error, an asynchronously generated error, or something else?

I moved it to the upper panel, to be visible in case console is full of other info messages (our case).
The colour is the same as for other error messages.

I made this to catch syntax errors.
On Jan 30, 2013, at 4:20 AM, Toby Ho notifications@github.com wrote:

For Dev mode I am not sure I like the way the error is represented in your pr: I think I like the old way better, which the top level error message at the bottom panel in red, the text in the tab should probably be in red though.

Just curious, when you encountered this problem, what kind of top-level-error was it? Was it like a syntax error, an asynchronously generated error, or something else?


Reply to this email directly or view it on GitHub.

Collaborator

airportyh commented Feb 1, 2013

@jankopriva I thought long and hard about this, but I want the top panel to represent the test results only: which is completely dependent on the test framework being used and does not take into account uncaught errors(I know we've been calling them top-level errors, but this is more clear).

For dev mode, I think what I'll do is to make all the top-level errors display a the top in the bottom panel. For ci mode, I am going to make it an option to bail on uncaught errors.

Collaborator

airportyh commented Feb 1, 2013

Added -b option for ci mode. See

testem ci --help

@airportyh airportyh closed this Feb 1, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment