Skip to content

Commit

Permalink
Core: Fix bug calling hooks with skipped tests
Browse files Browse the repository at this point in the history
When the first/last test in a module was skipped, it would cause the module's
before()/after() hook to not be called. This ensures those hooks will always be
called.
  • Loading branch information
Ben Demboski committed Apr 12, 2017
1 parent 2171bcf commit 42352e0
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 4 deletions.
13 changes: 9 additions & 4 deletions src/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,13 @@ export default function Test( settings ) {
this.callback = function() {};
this.async = false;
this.expected = 0;
} else {
this.assert = new Assert( this );
}

// even though skipped tests don't need an assert object, if they are
// the first/last test in their module, their assert property will be passed
// to the before/after module hook, so it needs to be set whether skipped or
// not.
this.assert = new Assert( this );
}

Test.count = 0;
Expand Down Expand Up @@ -203,8 +207,9 @@ Test.prototype = {
}
}

// Hooks are ignored on skipped tests
if ( !this.skip ) {
// Hooks are ignored on skipped tests, except the before/after module-level
// hooks.
if ( !this.skip || handler === "before" || handler === "after" ) {
processHooks( this, this.module );
}
return hooks;
Expand Down
52 changes: 52 additions & 0 deletions test/main/modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,33 @@ QUnit.test( "does not run before subsequent tests", function( assert ) {
assert.equal( this.beforeCount, 1, "beforeCount did not increase from last test" );
} );

QUnit.module( "before (skip)", {
before: function( assert ) {
assert.ok( true, "before hook ran" );

if ( typeof this.beforeCount === "undefined" ) {
this.beforeCount = 0;
}

this.beforeCount++;
}
} );

QUnit.skip( "skipped, but should still run before hook", function( assert ) {
assert.expect( 1 );
assert.ok( true );
} );

QUnit.test( "has run by the time second (unskipped) test runs", function( assert ) {
assert.expect( 1 );
assert.equal( this.beforeCount, 1, "beforeCount should be one" );
} );

QUnit.test( "does not run before subsequent tests", function( assert ) {
assert.expect( 1 );
assert.equal( this.beforeCount, 1, "beforeCount did not increase from last test" );
} );

QUnit.module( "after", {
after: function( assert ) {
assert.ok( true, "after hook ran" );
Expand All @@ -63,6 +90,31 @@ QUnit.test( "runs after final test", function( assert ) {
assert.expect( 1 );
} );

var ranAfterHook;

QUnit.module( "after (skip)", {
after: function( assert ) {
ranAfterHook = true;
assert.ok( true, "after hook ran" );
}
} );

QUnit.test( "does not run after initial tests", function( assert ) {
assert.expect( 0 );
ranAfterHook = false;
} );

QUnit.skip( "skipped, but should still run after hook", function( assert ) {
assert.expect( 1 );
} );

QUnit.module( "after (skip) verifier" );

QUnit.test( "after hook ran after last (skipped) test", function( assert ) {
assert.expect( 1 );
assert.ok( ranAfterHook, "after hook ran" );
} );

QUnit.module( "Test context object", {
beforeEach: function( assert ) {
var key,
Expand Down

0 comments on commit 42352e0

Please sign in to comment.