Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

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

Closed
Krinkle opened this Issue · 3 comments

3 participants

@Krinkle
Collaborator

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
@Jonahss

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

@jzaefferer jzaefferer closed this issue from a commit
@jzaefferer jzaefferer Tests: Remove outdated QUnit.reset test
This test isn't relevant anymore and causes odd failures when using
QUnit.log, since the failed assertion is logged, but ignored by the
HTML reporter.

Fixes gh-435
c94be7d
@jzaefferer
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.