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

Using QUnit.todo() causes build to fail instead of succeed #247

Open
Krinkle opened this issue Jun 6, 2021 · 0 comments · May be fixed by #249
Open

Using QUnit.todo() causes build to fail instead of succeed #247

Krinkle opened this issue Jun 6, 2021 · 0 comments · May be fixed by #249

Comments

@Krinkle
Copy link
Contributor

Krinkle commented Jun 6, 2021

Expected Behavior

Passing test.

Actual Behavior

[Windows 7, Firefox 45.0] Error: "my todo test" failed
Expected: true
  Actual: false

Details

This is happening because when browserstack-runner is consuming the testEnd event from js-reporters, it is ignoring the status field. Instead of only priting formatted errors for query.test.status === "failed", it is instead iterating over query.test.errors and always print each one as a formatted error, even if it is an expected error (such as the case with a "todo" test).

Here is the test suite for the linked example in the browser visually (passing):
capture

And here is what browserstack-runner report for that same build (failing):

https://github.com/qunitjs/qunit/runs/2753071016

[OS X Big Sur, Safari 14.0] Error: "test.todo.each [0]" failed
Expected: true
  Actual: false
  Source: runTest@http://localhost:8899/qunit/qunit.js:2262:41
	  run@http://localhost:8899/qunit/qunit.js:2250:16
	  http://localhost:8899/qunit/qunit.js:2472:19
	  processTaskQueue@http://localhost:8899/qunit/qunit.js:1860:37
	  http://localhost:8899/qunit/qunit.js:1864:29
	  promiseReactionJob@[native code]
Krinkle added a commit to qunitjs/browserstack-runner that referenced this issue Jun 6, 2021
Important changes:

* Mocha 8 and later are now supported. Previously, it's pending tests
  were incorrectly reported as undefined or failing.

* `QUnit.todo()` is now supported. Previously, these intentional
  failures were reported by browserstack-runner as failing tests.

  With js-reported 2.1 the runEnd report counts them as `todo` instead
  of as `failed`.

The API that browserstack-runner depends on has not changed since 1.1.0.
<https://github.com/js-reporters/js-reporters/blob/v2.1.0/CHANGELOG.md>.

Specifically, browserstack-runner captures the following data in
lib/_patch/reporter.js, and proccesses it in lib/server.js under
the "_progress" and "_report" handlers:

* The `testEnd` event, reading:
  - `name` string,
  - `suiteName` string,
  - `status` string,
  - `errors` array.

* The `runEnd` event, reading:
  - `status` string,
  - `testCounts` object with ints `total`, `passed`, `failed`, `skipped`,
  - `runtime` int.

These still exist the same way in the latest release:
<https://github.com/js-reporters/js-reporters/blob/v2.1.0/spec/cri-draft.adoc#runend>

Fixes browserstack#248.
Ref browserstack#247.
Krinkle added a commit to qunitjs/browserstack-runner that referenced this issue Jun 6, 2021
The update to js-reporters 2.1.0 in the previous commit has fixed
the most important problem, which is the build status reported
by the `runEnd` event, handled via "_report" submission.

However, the todo tests were still printed as errors in the output
which is confusing.

Before:

```
[Windows 8, Chrome 91.0] Error: "test.todo.each [0]" failed
  Expected: true
  Actual: false
[Windows 8, Chrome 91.0] Passed: 332 tests, 321 passed, 0 failed, 7 skipped; ran for 1255ms
```

After:

```
[Windows 10, Firefox 88.0] Passed: 332 tests, 321 passed, 0 failed, 7 skipped; ran for 1910ms
```

Fixes browserstack#247.
@Krinkle Krinkle linked a pull request Jun 6, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
1 participant