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

Ignored tests should not have a 'log' event emitted #435

Closed
Krinkle opened this Issue Mar 28, 2013 · 3 comments

Comments

Projects
None yet
3 participants
Owner

Krinkle commented Mar 28, 2013

When working on grunt-saucelabs I encountered an odd problem where there was one test failure reported through the log callback, yet both the 'done' callback report and the in-browser report showed no such failure.

Turns out that there is one scenario in which assertions happen outside the normal boundaries (between Test.init / testStart and Test.finish / testDone). Namely QUnit.reset.

In our own code test suite there is one test where this occurs:

(function() {
    var reset = QUnit.reset;
    module("reset");
    test("reset runs assertions", function() {
        expect(0);
        QUnit.reset = function() {
            ok( false, "reset should not modify test status" );
            reset.apply( this, arguments );
        };
    });
    test("reset runs assertions, cleanup", function() {
        expect(0);
        QUnit.reset = reset;
    });
})();

As expected, when Test.prototype.finish is called for the test named "reset runs assertions", it calls the QUnit.reset that is defined there.

There is a log event emitted for the ok() assertion.

However, because the DOM update is already done for this test, it is never displayed.

I'm not sure what the point of this test is and why we are purposely ignoring assertions within QUnit.reset, but it makes it hard to write a reporter that doesn't fail for QUnit's own test suite as this failure always shows up.

To work around it I made the reporter in qunit-saucelabs keep a buffer of assertions, and from the testDone callback, clear and ignore the buffer if obj.failed is 0.

@jzaefferer jzaefferer added this to the pre-2.0 milestone Feb 13, 2014

Jonahss commented Mar 10, 2014

Ha! grunt-saucelabs has been rewritten (to use Sauce's unit-test api) and this exact bug popped up again.

Owner

jzaefferer commented Mar 11, 2014

I actually ran into this recently as well, while working on browserstack-runner. Since QUnit.reset is deprecated and will be removed from the public API, having this test in place doesn't seem to be useful anymore. Since that's the only way to run into this issue, I've just gone ahead and removed the test.

Jonahss commented Mar 11, 2014

👍

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