Skip to content

Commit 4a2e830

Browse files
authored
Assert: Fix assert.timeout() bug causing a non-async test to fail
For an async test, we normally remember the assigned timeout amount, and the actual scheduling and enforcement of the timeout doesn't happen until after the test callback returns. If the test doesn't happen to be asynchronous, then the amount is discarded and we simply move on to the next test. Sometimes, the timeout ID stored in `config.timeout` for a previous async test would remain and thus caused `assert.timeout()` to wrongly think it was called twice in the same test, and tries clear and reschedule it, when actually it is clearing nothing and scheduling the first timeout and doing so before the test callback has returned. Thus when the test is over and we "ignore" the timeout, QUnit then got confused thinking it's an async test and thus waits until it times out. Fix this by making sure `config.timeout` is always explicitly emptied (or re-assigned) when a timeout is cancelled or has completed. Fixes #1539.
1 parent 9ab52be commit 4a2e830

File tree

2 files changed

+6
-7
lines changed

2 files changed

+6
-7
lines changed

src/assert.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ class Assert {
2525
// If a timeout has been set, clear it and reset with the new duration
2626
if ( config.timeout ) {
2727
clearTimeout( config.timeout );
28+
config.timeout = null;
2829

2930
if ( config.timeoutHandler && this.test.timeout > 0 ) {
3031
resetTestTimeout( this.test.timeout );

src/test.js

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -760,9 +760,9 @@ export function internalStop( test ) {
760760
}
761761

762762
if ( typeof timeoutDuration === "number" && timeoutDuration > 0 ) {
763-
clearTimeout( config.timeout );
764763
config.timeoutHandler = function( timeout ) {
765764
return function() {
765+
config.timeout = null;
766766
pushFailure(
767767
`Test took longer than ${timeout}ms; test timed out.`,
768768
sourceFromStacktrace( 2 )
@@ -771,6 +771,7 @@ export function internalStop( test ) {
771771
internalRecover( test );
772772
};
773773
};
774+
clearTimeout( config.timeout );
774775
config.timeout = setTimeout(
775776
config.timeoutHandler( timeoutDuration ),
776777
timeoutDuration
@@ -827,17 +828,14 @@ function internalStart( test ) {
827828

828829
// Add a slight delay to allow more assertions etc.
829830
if ( setTimeout ) {
830-
if ( config.timeout ) {
831-
clearTimeout( config.timeout );
832-
}
831+
clearTimeout( config.timeout );
833832
config.timeout = setTimeout( function() {
834833
if ( test.semaphore > 0 ) {
835834
return;
836835
}
837836

838-
if ( config.timeout ) {
839-
clearTimeout( config.timeout );
840-
}
837+
clearTimeout( config.timeout );
838+
config.timeout = null;
841839

842840
begin();
843841
} );

0 commit comments

Comments
 (0)