Test: Improve assert.pushResult() tests

History.md

@@ -44,7 +44,6 @@ QUnit 3.0 Roadmap and feedback: https://github.com/qunitjs/qunit/issues/1498
 * Core: Remove deprecated [`QUnit.extend()`](https://qunitjs.com/api/extension/QUnit.extend/). (Izel Nakri)
 * Core: Remove deprecated [`QUnit.load()`](https://qunitjs.com/api/QUnit/load/). [#1084](https://github.com/qunitjs/qunit/issues/1084)
 * Core: Remove deprecated `QUnit.onError()` and `QUnit.onUnhandledRejection()` in favor of [QUnit.onUncaughtException()](https://qunitjs.com/api/extension/QUnit.onUncaughtException/).
-* Assert: Remove deprecated [`assert.push()`](https://qunitjs.com/api/assert/push/).
 * Core: Remove undocumented `details.modules[].tests` from QUnit.begin() event.
 * HTML Reporter: Remove support for legacy markup, use `<div id="qunit">` instead. Check [Browser Runner § Getting started](https://qunitjs.com/browser/).
 * Build: Discontinue publication to Bower for future releases. Check [How to install](https://qunitjs.com/intro/#download) or [Getting started](https://qunitjs.com/intro/). [#1677](https://github.com/qunitjs/qunit/issues/1677)

src/assert.js

@@ -1,6 +1,5 @@
 import dump from './dump.js';
 import equiv from './equiv.js';
-
 import config from './core/config.js';
 import { objectType, objectValues, objectValuesSubset, errorString } from './core/utilities.js';
 import { sourceFromStacktrace } from './core/stacktrace.js';
@@ -82,8 +81,8 @@ class Assert {
   }
 
   push (result, actual, expected, message, negative) {
-    const currentAssert = this instanceof Assert ? this : config.current.assert;
-    return currentAssert.pushResult({
+    const currentTest = (this instanceof Assert && this.test) || config.current;
+    return currentTest.pushResult({
       result,
       actual,
       expected,
@@ -94,24 +93,18 @@ class Assert {
 
   // Public API to internal test.pushResult()
   pushResult (resultInfo) {
-    // Destructure of resultInfo = { result, actual, expected, message, negative }
-    let assert = this;
-    const currentTest = (assert instanceof Assert && assert.test) || config.current;
-
-    // Backwards compatibility fix.
-    // Allows the direct use of global exported assertions and QUnit.assert.*
-    // Although, it's use is not recommended as it can leak assertions
-    // to other tests from async tests, because we only get a reference to the current test,
-    // not exactly the test where assertion were intended to be called.
+    // Prefer context object when possible, so that test.pushResult() can provide
+    // useful error when producing assertions on a closed test.
+    const currentTest = (this instanceof Assert && this.test) || config.current;
+
+    // Backwards compatibility for direct use of global QUnit.assert.* methods.
+    // It's use is not recommended as it can leak assertions to other tests from async
+    // tests, because we only get a reference to the current test.
     if (!currentTest) {
       throw new Error('assertion outside test context, in ' + sourceFromStacktrace(2));
     }
 
-    if (!(assert instanceof Assert)) {
-      assert = currentTest.assert;
-    }
-
-    return assert.test.pushResult(resultInfo);
+    return currentTest.pushResult(resultInfo);
   }
 
   ok (result, message) {

test/main/legacy.js

@@ -18,10 +18,17 @@
     QUnit.assert.raises(function () {
       throw new Error('boo');
     }, /boo/);
+    QUnit.assert.push(true, 1, 1, 'hi');
+    QUnit.assert.pushResult({
+      result: true,
+      actual: 1,
+      expected: 1,
+      message: 'hello'
+    });
   }
 
   QUnit.test('global QUnit.assert calls', function (assert) {
-    assert.expect(12);
+    assert.expect(14);
     helper();
   });
 

test/main/test.js

@@ -133,12 +133,10 @@ QUnit.module('test', function () {
     this.push(true, value, expected, message, false);
   };
 
-  QUnit.test('mod2', function (assert) {
+  QUnit.test('assert.pushResult()', function (assert) {
     var detail;
     QUnit.log(function (data) {
-      if (data.message === 'three') {
-        detail = data;
-      }
+      detail = data;
     });
 
     assert.mod2(2, 0, 'two');
@@ -153,6 +151,23 @@ QUnit.module('test', function () {
     });
   });
 
+  QUnit.test('assert.push()', function (assert) {
+    var detail;
+    QUnit.log(function (data) {
+      detail = data;
+    });
+
+    assert.testForPush(10, 20, 'hello');
+
+    assert.propContains(detail, {
+      result: true,
+      actual: 10,
+      expected: 20,
+      message: 'hello',
+      negative: false
+    });
+  });
+
   QUnit.module('aliases');
 
   ['todo', 'skip', 'only'].forEach(function (flavor) {