[hoanle0126]

What & why

Fixes #459

In Nightwatch, testcase.passed and testcase.failed count Nightwatch browser.assert.* / browser.verify.* assertions inside that testcase. When a test runs and finishes without making a Nightwatch assertion (or uses node:assert), Nightwatch records { passed: 0, failed: 0, errors: 0, skipped: 0 } in results.testcases.

Previously, determineTestState(testcase) checked if (testcase.passed === 0 && testcase.failed === 0) return 'skipped', which marked any passing test without Nightwatch assertions as skipped (and also ignored testcase.errors).

This updates determineTestState(testcase) to return 'failed' when testcase.failed > 0 || testcase.errors > 0 and 'passed' for completed testcases without Nightwatch assertions, and removes the KNOWN_DEBT.nightwatchSkipped conformance allowlist entry.

Type of change

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Polish (an improvement to an existing feature)
• Breaking change (existing behavior changes for users)
• Documentation
• Internal (build, CI, dependencies, tooling)

Packages touched

• shared (types and contracts)
• core (framework-agnostic capture/reporting)
• elements (published element/snapshot API — @wdio/elements)
• service (WebdriverIO adapter)
• nightwatch-devtools (Nightwatch adapter)
• selenium-devtools (Selenium adapter)
• selenium-devtools-py (Selenium Python adapter)
• backend (server)
• app (UI)
• script (page-injected runtime)
• trace (Trace mode)

Notes for reviewers

• Tests: Updated packages/nightwatch-devtools/tests/utils.test.ts ({ passed: 0, failed: 0, errors: 0, skipped: 0 } → 'passed', { passed: 0, failed: 0, errors: 1, skipped: 0 } → 'failed', and { passed: 1, failed: 0, errors: 1, skipped: 0 } → 'failed') and removed KNOWN_DEBT.nightwatchSkipped from packages/conformance.
• Manual verification: Not run manually against examples/nightwatch in a live browser; validated via the unit tests and the conformance suite (RULES.testsAsDesigned).

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no blocking issues were found.

Summary

Updates Nightwatch test results so completed tests without Nightwatch assertions report passed, while tests with errors report failed.

• Nightwatch test results now reflect recorded failures and skipped counts.

_{Reviews (3) · Last reviewed commit: "Merge origin/main into fix/nightwatch-pa..." · Reviewed by Greptile}

[vishnuv688]

Thanks for this, the fix is correct and the right size. I checked it against Nightwatch 3.16's reporter (lib/reporter/results.js): a test only gets a results.testcases entry from setCurrentTest, when it actually runs, and tests skipped by the user or at runtime are tracked in separate lists. So passed: 0, failed: 0 really is a test that ran without a Nightwatch assertion, and reporting it passed is right. Counting errors > 0 as failed is a good catch too. The new cases cover the regression.

A few small things before merge:

1. Drop the ?? 0 guards. NightwatchTestCase types errors and skipped as required numbers, and createTestCaseResults always sets both to 0. Guarding those two but not passed/failed suggests they can be missing when neither the type nor Nightwatch says so. Plain testcase.errors > 0 and testcase.skipped > 0 read better.
2. The skipped branch is unreachable in practice. Nightwatch never increments the per-testcase skipped counter, so a real skip is decided by the callers that find no testcase entry (closePreviousTest.ts, test-lifecycle.ts), not here. Keeping it as a defensive branch is fine, or you could drop it so the function is just failed-or-errors, else passed. Either way, the description shouldn't suggest this function now handles skips.
3. Changeset wording: we write changesets as current behaviour rather than before/after, and leave out internal counters. Something like: "A Nightwatch test that makes no Nightwatch assertion is reported as passed, and one with errors as failed."
4. Optional test: a case with errors > 0 alongside passed > 0, to pin that a failure wins.

Did you get a chance to run it against examples/nightwatch (a test with only node:assert, or no assertions, showing green in the dashboard sidebar)? If not, mentioning that in the PR description is enough. Our cross-adapter conformance suite (packages/conformance) currently allowlists this bug as a known gap, so once this merges that gap will be reported stale and we'll remove it on our side.

[vishnuv688]

One correction to my last point: since #465 merged, main's conformance suite (part of pnpm test) allowlists this bug, and a gap that stops occurring fails the run. After rebasing on main, please also delete the Nightwatch entry with reason: KNOWN_DEBT.nightwatchSkipped in packages/conformance/src/allowlist.ts, the nightwatchSkipped entry in KNOWN_DEBT (packages/conformance/src/constants.ts), and the "Nightwatch reports a passing test skipped (#459)" bullet under Known debt in CLAUDE.md. The suite going green without them is the proof the fix works.

[hoanle0126]

Thanks for the detailed review @vishnuv688! Rebased onto main and updated all items:

• Dropped the ?? 0 guards in determineTestState.
• Added the { passed: 1, failed: 0, errors: 1, skipped: 0 } -> 'failed' test case.
• Updated the changeset wording and PR description.
• Removed KNOWN_DEBT.nightwatchSkipped from packages/conformance/src/allowlist.ts, packages/conformance/src/constants.ts, and CLAUDE.md.

[vishnuv688]

Thanks for the quick turnaround, @hoanle0126, every review point is addressed. Main moved on after this opened (#473, #474 and #475 changed the same conformance files), so I merged main into your branch in 4ba4f23 rather than ask you to. Your two commits are untouched. The resolution: with the #459 gap gone, reportedAs and the REFUSED scenario title had no other user, so both are deleted, and the import drops failureUnexplained, which main had already removed.

I also checked that your change is what the suite now relies on: with determineTestState reverted, the conformance run fails in all three BiDi modes with "is reported skipped", and with it the full suite passes (3035 tests, conformance 15/15 twice). If you push again, please pull first so you don't hit a conflict on your side.

[vishnuv688]

LGTM

[vishnuv688]

@hoanle0126 Let me know if there is no other changes I will merge this PR.

[hoanle0126]

No other changes from my side, it's ready to merge. Thanks @vishnuv688!