Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TAP reporter prints 'Infinity' in a failing test as 'null' #1406

Closed
mathiasbockwoldt opened this issue Sep 23, 2019 · 7 comments · Fixed by #1577
Closed

TAP reporter prints 'Infinity' in a failing test as 'null' #1406

mathiasbockwoldt opened this issue Sep 23, 2019 · 7 comments · Fixed by #1577

Comments

@mathiasbockwoldt
Copy link

Tell us about your runtime:

  • QUnit version: 2.9.2
  • What environment are you running QUnit in?: Node
  • How are you running QUnit?: Linux shell (qunit my_test_file.js; Linux Mint 19.1; Kernel v. 4.15.0)

What are you trying to do?

Code that reproduces the problem:

// my_test_file.js
QUnit.test('Infinity test', function(assert) {
    assert.strictEqual(null, Infinity, 'Comparing null and Infinity');
});

Run using qunit my_test_file.js.

What did you expect to happen?

I expect to see an error that should look approximately like this. Note the expected: Infinity.

not ok 1 Infinity test
  ---
  message: "Comparing null and Infinity"
  severity: failed
  actual: null
  expected: Infinity
  stack:     [...]
  ...

What actually happened?

Note the expected: null.

not ok 1 Infinity test
  ---
  message: "Comparing null and Infinity"
  severity: failed
  actual: null
  expected: null
  stack:     [...]
  ...

I don't know much about the internals of QUnit, but this smells JSON conversion.

@mathiasbockwoldt
Copy link
Author

I researched a little bit more. Not only Infinity is shown as null, but also -Infinity and NaN. undefined is ok, though (somewhat expected).

// my_test_file.js
QUnit.test('null tests', function(assert) {
	assert.strictEqual(null, Infinity, 'null vs Infinity');
	assert.strictEqual(null, -Infinity, 'null vs -Infinity');
	assert.strictEqual(null, NaN, 'null vs NaN');
	assert.strictEqual(null, undefined, 'null vs undefined');
	assert.strictEqual(null, 0, 'null vs 0');
});

Running qunit my_test_file.js yiels:

TAP version 13
not ok 1 null tests
  ---
  message: "null vs Infinity"
  severity: failed
  actual: null
  expected: null  // should be Infinity
  stack:     [...]
  ...
  ---
  message: "null vs -Infinity"
  severity: failed
  actual: null
  expected: null  // should be -Infinity
  stack:     [...]
  ...
  ---
  message: "null vs NaN"
  severity: failed
  actual: null
  expected: null  // should be NaN
  stack:     [...]
  ...
  ---
  message: "null vs undefined"
  severity: failed
  actual: null
  expected: undefined  // that's ok
  stack:     [...]
  ...
  ---
  message: "null vs 0"
  severity: failed
  actual: null
  expected: 0 // this is of course also ok
  stack:     [...]
  ...
1..1
[...]

@platinumazure
Copy link
Contributor

Hi @mathiasbockwoldt, thanks for the issue.

Is this only showing up in the TAP reporter? Do you know if the same issue shows up in the HTML reporter?

@mathiasbockwoldt
Copy link
Author

I just tried it and it looks like it shows as expected in the HTML reporter. I used the example code from qunitjs.com and the test code from my last comment.

null_tests_html

@platinumazure
Copy link
Contributor

Thank you @mathiasbockwoldt for doing that test and helping us narrow the issue down to the TAP reporter!

@Krinkle
Copy link
Member

Krinkle commented Jun 20, 2020

I suspect this likely due to a JSON serialisation step happening somewhere between QUnit and the TAP reporter we use.

Complex number values like Infinity and NaN are not supported in JSON.

Having said that, since there is no cross-process communication (I think?) there should not be a need to serialise these.

@Krinkle Krinkle changed the title Infinity is shown as null in error report TAP reporter prints 'Infinity' in a failing test as 'null' Jun 20, 2020
@Krinkle
Copy link
Member

Krinkle commented Aug 12, 2020

This might be related to js-reporters/js-reporters#110. Based on that, it seems TAP wants to be given strings to display, which means we may need to stringify these within QUnit first and thus not expose to reporters at all as the native values that they are.

@rwjblue Does that sound right?

@Krinkle
Copy link
Member

Krinkle commented Nov 7, 2020

This was fixed upstream by js-reporters/js-reporters@7ec72e3. I've added a test for it in js-reporters/js-reporters@31ab6e0fbf849bd0.

The issue was indeed the way the TapReporter converted JS values to JSON/YAML.

This will apply to QUnit after the next js-reporters release.

Krinkle added a commit to Krinkle/qunit that referenced this issue Apr 4, 2021
Krinkle added a commit that referenced this issue Apr 9, 2021
Krinkle added a commit that referenced this issue Apr 9, 2021
Krinkle added a commit to Krinkle/qunit that referenced this issue Apr 9, 2021
Krinkle added a commit that referenced this issue Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

4 participants