Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix for missing traces on test failure if using Phantom.js #291

Closed
wants to merge 1 commit into from

10 participants

@jphpsf

Hi

I've started to run my Jasmine tests using Phantom.js and I noticed that I was not getting a stacktrace for the expectation failures, which makes debugging a test failure harder. After some research, I found a similar issue on Stackoverflow. After applying the patch, I get the exception trace as expected. I thought I'll submit the patch as a pull request as other might encounter the issue.

I tested in Chrome and Firefox and still see a stack trace there, so that's good. I also ran thor jasmine_dev:execute_specs and did not see any failures. Please let me know what you think :)

Before the patch, no trace:

$ rake phantomjs
phantomjs lib/phantom-jasmine/run_jasmine_test.coffee SpecRunner.html
Starting...
when song has been paused : should be possible to resume
Error: Expected false to be truthy.

when song has been paused: 3 of 4 passed.
#resume: 1 of 1 passed.

Finished
-----------------
5 specs, 1 failure in 0.087s.

After the patch, here is our trace:

$ rake phantomjs
phantomjs lib/phantom-jasmine/run_jasmine_test.coffee SpecRunner.html
Starting...
when song has been paused : should be possible to resume
Error: Expected false to be truthy.
    at file:///home/jpc/public_html/jasmine-phantomjs/lib/jasmine-1.2.0/jasmine.js:103
    at file:///home/jpc/public_html/jasmine-phantomjs/lib/jasmine-1.2.0/jasmine.js:1201
    at file:///home/jpc/public_html/jasmine-phantomjs/spec/PlayerSpec.js:33
    at file:///home/jpc/public_html/jasmine-phantomjs/lib/jasmine-1.2.0/jasmine.js:1026
    at file:///home/jpc/public_html/jasmine-phantomjs/lib/jasmine-1.2.0/jasmine.js:2027
    at file:///home/jpc/public_html/jasmine-phantomjs/lib/jasmine-1.2.0/jasmine.js:1980
    at file:///home/jpc/public_html/jasmine-phantomjs/lib/jasmine-1.2.0/jasmine.js:2309
    at file:///home/jpc/public_html/jasmine-phantomjs/lib/jasmine-1.2.0/jasmine.js:2027
    at file:///home/jpc/public_html/jasmine-phantomjs/lib/jasmine-1.2.0/jasmine.js:1980
    at file:///home/jpc/public_html/jasmine-phantomjs/lib/jasmine-1.2.0/jasmine.js:2454
    at file:///home/jpc/public_html/jasmine-phantomjs/lib/jasmine-1.2.0/jasmine.js:2027
    at file:///home/jpc/public_html/jasmine-phantomjs/lib/jasmine-1.2.0/jasmine.js:2023
    at file:///home/jpc/public_html/jasmine-phantomjs/lib/jasmine-1.2.0/jasmine.js:2281
    at file:///home/jpc/public_html/jasmine-phantomjs/lib/jasmine-1.2.0/jasmine.js:2308
    at file:///home/jpc/public_html/jasmine-phantomjs/lib/jasmine-1.2.0/jasmine.js:2037
    at file:///home/jpc/public_html/jasmine-phantomjs/lib/jasmine-1.2.0/jasmine.js:2017

when song has been paused: 3 of 4 passed.
#resume: 1 of 1 passed.

Finished
-----------------
5 specs, 1 failure in 0.076s.
@jamesarosen

Love it.

@dwt
dwt commented

+1

@JamesMGreene

Well, actually, I would probably advise rewriting this so that it doesn't throw an Error unnecessarily (not always a light hit) if the tests passed in the first place. For example:

this.trace = this.passed_ ? '' : (function() {
    var trace = params.trace;
    if (!trace) {
        try {
            throw new Error(this.message);
        }
        catch(e) {
            trace = e;
        }
    }
    return trace;
})();
@r4j4h

I think the ending semicolon is a little off, I think it should be
try { throw new Error(this.message); } catch(e) { err = e; }

@r4j4h

Great improvement, James!

(if you saw my other stuff here, I'm silly, the lines are changing from my changing of the code - one solution is 3 additional lines but the other adds about 8 or so which is why those were changing. Silly me)

@JamesMGreene

Adding comment directly to the commit to ensure it is seen merging.
I'd love to see this fixed, too, but I would probably advise rewriting this so that it doesn't throw an Error unnecessarily (not always a light hit) if the tests passed in the first place. For example:

this.trace = this.passed_ ? '' : (function() {
    var trace = params.trace;
    if (!trace) {
        try {
            throw new Error(this.message);
        }
        catch(e) {
            trace = e;
        }
    }
    return trace;
})();

Thanks for your comment. I will look into it soon and improve the patch.

@dwt

Same comment here, if you stop on any exception in your debugger to catch that pesky thing that breaks your code but that is then catched and processed by the testing framework, it's greatly annoying if every test throws and catches an exception.

@jphpsf

Thank you for your comments everyone :) I will look into it soon and improve the patch.

@prashantrajan

What's the status of this?

@infews
Owner

@ariya can you give us a more direct way of getting the stack trace out of Phantom?

@JamesMGreene

@infews What would you propose? Seems odd if you're implying you'd rather call some PhantomJS-specific method rather than generating a stack trace with this relatively generic mechanism.

It's also possible, of course, that this issue will be resolved when we (PhantomJS) upgrade our version of WebKit to the latest bug fix point release (4.8.4) or the latest major release (5.0).

@ariya

Like @JamesMGreene, currently there is no way to get the stack trace unless you throw an exception. Maybe in the near future the situation would change, but that's what we have now.

@infews
Owner

Fascinated. We notice that depending on the browser involved, we get completely different properties on the exception object that is caught. Firefox/Safari/Chrome/Node are all different. The current tests say they're catching Webkit and Firefox, but I noticed that we need to be even more tolerant of the different props we get. What properties are on the e in Phantom? And why are they different in one catch versus another?

@thurloat

You could use window.callPhantom in your reporter and page.onCallback to eat them up in Phantom and report on it however you want. It would probably require re-writing your phantom runner completely to support it, but I explain how I set it up in a little more depth on my blog here: http://thurloat.com/2012/12/10/mocha-phantomjs-jasmine-reporting if anyone is interested in checking it out.

@infews
Owner

Those of you keeping score at home I can clarify what Jasmine's exception formatting does (see: https://github.com/pivotal/jasmine/blob/master/spec/core/ExceptionFormatterSpec.js). The stack trace is either present (Chrome/V8/Node/FF) or not (Safari). The other properties from which one can build a nice message differ.

So I would expect error.stack to always give a String that's the stack trace.

@slackersoft
Owner

We think this should be better for 2.0. Check out master of the jasmine-gem and let us know if it is still an issue.

@slackersoft slackersoft closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 3 additions and 1 deletion.
  1. +3 −1 lib/jasmine-core/jasmine.js
View
4 lib/jasmine-core/jasmine.js
@@ -106,7 +106,9 @@ jasmine.ExpectationResult = function(params) {
this.actual = params.actual;
this.message = this.passed_ ? 'Passed.' : params.message;
- var trace = (params.trace || new Error(this.message));
+ var err;
+ try { throw new Error(this.message); } catch(e) { err = e };
+ var trace = (params.trace || err);
this.trace = this.passed_ ? '' : trace;
};
Something went wrong with that request. Please try again.