CLI: Remove confusing expected: undefined from TAP for non-assert fail

Cherry-picked from 9a1fc05 (3.0.0-dev). This also includes a partial cherry-pick of 0f7aeac (3.0.0-dev), which removes an unused `actual` argument from the pushFailure function. Ref #1794.
qunitjs · Dec 4, 2024 · fa7b888 · fa7b888
1 parent 77a3a2a
commit fa7b888
Show file tree

Hide file tree

Showing 22 changed files with 20 additions and 57 deletions.
diff --git a/src/core/on-uncaught-exception.js b/src/core/on-uncaught-exception.js
@@ -25,6 +25,7 @@ import { emit } from '../events';
  */
 export default function onUncaughtException (error) {
   if (config.current) {
+    // This omits 'actual' and 'expected' (undefined)
     config.current.assert.pushResult({
       result: false,
       message: `global failure: ${errorString(error)}`,

diff --git a/src/html-reporter/html.js b/src/html-reporter/html.js
@@ -861,10 +861,13 @@ const stats = {
     let diff;
     let showDiff = false;
 
-    // The pushFailure doesn't provide details.expected
-    // when it calls, it's implicit to also not show expected and diff stuff
-    // Also, we need to check details.expected existence, as it can exist and be undefined
-    if (!details.result && hasOwn.call(details, 'expected')) {
+    // When pushFailure() is called, it is implied that no expected value
+    // or diff should be shown, because both expected and actual as undefined.
+    //
+    // This must check details.expected existence. If it exists as undefined,
+    // that's a regular assertion for which to render actual/expected and a diff.
+    const showAnyValues = !details.result && (details.expected !== undefined || details.actual !== undefined);
+    if (showAnyValues) {
       if (details.negative) {
         expected = 'NOT ' + QUnit.dump.parse(details.expected);
       } else {

diff --git a/src/reporters/TapReporter.js b/src/reporters/TapReporter.js
@@ -1,7 +1,6 @@
 import kleur from 'kleur';
 import { errorString } from '../core/utilities';
 import { console } from '../globals';
-const hasOwn = Object.prototype.hasOwnProperty;
 
 /**
  * Format a given value into YAML.
@@ -253,11 +252,12 @@ export default class TapReporter {
     out += `\n  message: ${prettyYamlValue(error.message || 'failed')}`;
     out += `\n  severity: ${prettyYamlValue(severity || 'failed')}`;
 
-    if (hasOwn.call(error, 'actual')) {
+    // When pushFailure() is used, actual/expected are initially unset but
+    // eventually in Test#logAssertion, for testReport#pushAssertion, these are
+    // forged into existence as undefined.
+    const hasAny = (error.expected !== undefined || error.actual !== undefined);
+    if (hasAny) {
       out += `\n  actual  : ${prettyYamlValue(error.actual)}`;
-    }
-
-    if (hasOwn.call(error, 'expected')) {
       out += `\n  expected: ${prettyYamlValue(error.expected)}`;
     }
 

diff --git a/src/test.js b/src/test.js
@@ -621,7 +621,7 @@ Test.prototype = {
     });
   },
 
-  pushFailure: function (message, source, actual) {
+  pushFailure: function (message, source) {
     if (!(this instanceof Test)) {
       throw new Error('pushFailure() assertion outside test context, was ' +
         sourceFromStacktrace(2));
@@ -630,7 +630,6 @@ Test.prototype = {
     this.pushResult({
       result: false,
       message: message || 'error',
-      actual: actual || null,
       source
     });
   },

diff --git a/test/cli/cli-main.js b/test/cli/cli-main.js
@@ -37,8 +37,6 @@ not ok 2 slow
   ---
   message: Test took longer than 7ms; test timed out.
   severity: failed
-  actual  : null
-  expected: undefined
   stack: |
         at internal
   ...

diff --git a/test/cli/fixtures/assert-expect-failure-step.tap.txt b/test/cli/fixtures/assert-expect-failure-step.tap.txt
@@ -6,8 +6,6 @@ not ok 3 wrong [a little off]
   ---
   message: Expected 2 assertions, but 1 were run
   severity: failed
-  actual  : null
-  expected: undefined
   stack: |
         at /qunit/test/cli/fixtures/assert-expect-failure-step.js:25:7
         at internal
@@ -16,8 +14,6 @@ not ok 4 wrong [way off]
   ---
   message: Expected 5 assertions, but 1 were run
   severity: failed
-  actual  : null
-  expected: undefined
   stack: |
         at /qunit/test/cli/fixtures/assert-expect-failure-step.js:33:7
         at internal
@@ -28,8 +24,6 @@ not ok 5 previously passing [once]
     Expected 4 assertions, but 2 were run
     Remember that with QUnit.config.countStepsAsOne and in QUnit 3.0, steps no longer count as separate assertions. https://qunitjs.com/api/assert/expect/
   severity: failed
-  actual  : null
-  expected: undefined
   stack: |
         at /qunit/test/cli/fixtures/assert-expect-failure-step.js:43:7
         at internal
@@ -40,8 +34,6 @@ not ok 6 previously passing [twice]
     Expected 9 assertions, but 4 were run
     Remember that with QUnit.config.countStepsAsOne and in QUnit 3.0, steps no longer count as separate assertions. https://qunitjs.com/api/assert/expect/
   severity: failed
-  actual  : null
-  expected: undefined
   stack: |
         at /qunit/test/cli/fixtures/assert-expect-failure-step.js:52:7
         at internal

diff --git a/test/cli/fixtures/assert-expect-failure.tap.txt b/test/cli/fixtures/assert-expect-failure.tap.txt
@@ -6,8 +6,6 @@ not ok 1 failing test
   ---
   message: Expected 2 assertions, but 1 were run
   severity: failed
-  actual  : null
-  expected: undefined
   stack: |
         at /qunit/test/cli/fixtures/assert-expect-failure.js:1:7
         at internal

diff --git a/test/cli/fixtures/assert-expect-no-assertions.tap.txt b/test/cli/fixtures/assert-expect-no-assertions.tap.txt
@@ -6,8 +6,6 @@ not ok 1 no assertions
   ---
   message: Expected at least one assertion, but none were run - call expect(0) to accept zero assertions.
   severity: failed
-  actual  : null
-  expected: undefined
   stack: |
         at /qunit/test/cli/fixtures/assert-expect-no-assertions.js:1:7
         at internal

diff --git a/test/cli/fixtures/config-noglobals-add.tap.txt b/test/cli/fixtures/config-noglobals-add.tap.txt
@@ -6,8 +6,6 @@ not ok 1 adds global var
   ---
   message: Introduced global variable(s): dummyGlobal
   severity: failed
-  actual  : null
-  expected: undefined
   stack: |
         at qunit.js
   ...

diff --git a/test/cli/fixtures/config-noglobals-remove.tap.txt b/test/cli/fixtures/config-noglobals-remove.tap.txt
@@ -6,8 +6,6 @@ not ok 1 deletes global var
   ---
   message: Deleted global variable(s): dummyGlobal
   severity: failed
-  actual  : null
-  expected: undefined
   stack: |
         at qunit.js
   ...
@@ -17,4 +15,4 @@ not ok 1 deletes global var
 # todo 0
 # fail 1
 
-# exit code: 1
+# exit code: 1
diff --git a/test/cli/fixtures/config-requireExpects.tap.txt b/test/cli/fixtures/config-requireExpects.tap.txt
@@ -6,8 +6,6 @@ not ok 1 passing test
   ---
   message: Expected number of assertions to be defined, but expect() was not called.
   severity: failed
-  actual  : null
-  expected: undefined
   stack: |
         at /qunit/test/cli/fixtures/config-requireExpects.js:3:7
         at internal
@@ -18,4 +16,4 @@ not ok 1 passing test
 # todo 0
 # fail 1
 
-# exit code: 1
+# exit code: 1
diff --git a/test/cli/fixtures/config-testTimeout.tap.txt b/test/cli/fixtures/config-testTimeout.tap.txt
@@ -6,8 +6,6 @@ not ok 1 slow
   ---
   message: Test took longer than 10ms; test timed out.
   severity: failed
-  actual  : null
-  expected: undefined
   stack: |
         at internal
   ...

diff --git a/test/cli/fixtures/done-after-timeout.tap.txt b/test/cli/fixtures/done-after-timeout.tap.txt
@@ -6,8 +6,6 @@ not ok 1 times out before scheduled done is called
   ---
   message: Test took longer than 10ms; test timed out.
   severity: failed
-  actual  : null
-  expected: undefined
   stack: |
         at internal
   ...
@@ -17,4 +15,4 @@ not ok 1 times out before scheduled done is called
 # todo 0
 # fail 1
 
-# exit code: 1
+# exit code: 1
diff --git a/test/cli/fixtures/drooling-done.tap.txt b/test/cli/fixtures/drooling-done.tap.txt
@@ -9,8 +9,6 @@ not ok 1 Test A
         at /qunit/test/cli/fixtures/drooling-done.js:5:7
         at internal
   severity: failed
-  actual  : null
-  expected: undefined
   stack: |
     Error: this is an intentional error
         at /qunit/test/cli/fixtures/drooling-done.js:8:9

diff --git a/test/cli/fixtures/drooling-extra-done.tap.txt b/test/cli/fixtures/drooling-extra-done.tap.txt
@@ -11,8 +11,6 @@ not ok 2 Test B
         at /qunit/test/cli/fixtures/drooling-extra-done.js:13:7
         at internal
   severity: failed
-  actual  : null
-  expected: undefined
   stack: |
     Error: Unexpected release of async pause during a different test.
     > Test: Test A [async #1]

diff --git a/test/cli/fixtures/hanging-test.tap.txt b/test/cli/fixtures/hanging-test.tap.txt
@@ -8,4 +8,4 @@ Test "hanging" took longer than 3000ms, but no timeout was set. Set QUnit.config
 Error: Process exited before tests finished running
 Last test to run (hanging) has an async hold. Ensure all assert.async() callbacks are invoked and Promises resolve. You should also set a standard timeout via QUnit.config.testTimeout.
 
-# exit code: 1
+# exit code: 1
diff --git a/test/cli/fixtures/pending-async-after-timeout.tap.txt b/test/cli/fixtures/pending-async-after-timeout.tap.txt
@@ -1,13 +1,11 @@
 # name: test with pending async after timeout
-# command: ["qunit","pending-async-after-timeout.js"]
+# command: ["qunit", "pending-async-after-timeout.js"]
 
 TAP version 13
 not ok 1 example
   ---
   message: Test took longer than 10ms; test timed out.
   severity: failed
-  actual  : null
-  expected: undefined
   stack: |
         at internal
   ...
@@ -17,4 +15,4 @@ not ok 1 example
 # todo 0
 # fail 1
 
-# exit code: 1
+# exit code: 1
diff --git a/test/cli/fixtures/timeout.tap.txt b/test/cli/fixtures/timeout.tap.txt
@@ -6,8 +6,6 @@ not ok 1 timeout > first
   ---
   message: Test took longer than 10ms; test timed out.
   severity: failed
-  actual  : null
-  expected: undefined
   stack: |
         at internal
   ...

diff --git a/test/cli/fixtures/too-many-done-calls.tap.txt b/test/cli/fixtures/too-many-done-calls.tap.txt
@@ -10,8 +10,6 @@ not ok 1 Test A
         at /qunit/test/cli/fixtures/too-many-done-calls.js:1:7
         at internal
   severity: failed
-  actual  : null
-  expected: undefined
   stack: |
     Error: Tried to release async pause that was already released.
     > Test: Test A [async #1]

diff --git a/test/cli/fixtures/uncaught-error-after-assert-async.tap.txt b/test/cli/fixtures/uncaught-error-after-assert-async.tap.txt
@@ -9,8 +9,6 @@ not ok 1 contains a hard error after using assert.async()
         at /qunit/test/cli/fixtures/uncaught-error-after-assert-async.js:1:7
         at internal
   severity: failed
-  actual  : null
-  expected: undefined
   stack: |
     Error: expected error thrown in test
         at /qunit/test/cli/fixtures/uncaught-error-after-assert-async.js:4:9

diff --git a/test/cli/fixtures/uncaught-error-in-hook.tap.txt b/test/cli/fixtures/uncaught-error-in-hook.tap.txt
@@ -6,8 +6,6 @@ not ok 1 contains a hard error in hook > contains a hard error
   ---
   message: before failed on contains a hard error: expected error thrown in hook
   severity: failed
-  actual  : null
-  expected: undefined
   stack: |
     Error: expected error thrown in hook
         at /qunit/test/cli/fixtures/uncaught-error-in-hook.js:3:11

diff --git a/test/cli/fixtures/unhandled-rejection.tap.txt b/test/cli/fixtures/unhandled-rejection.tap.txt
@@ -18,8 +18,6 @@ not ok 2 Unhandled Rejections > test passes just fine, but has a rejected promis
   ---
   message: global failure: Error: Error thrown in non-returned promise!
   severity: failed
-  actual  : undefined
-  expected: undefined
   stack: |
     Error: Error thrown in non-returned promise!
         at /qunit/test/cli/fixtures/unhandled-rejection.js:10:13