Skip to content

fix(observability,session-status): don't mark suppressNotFoundErrors commands as test failures#53

Merged
shubhamkd merged 2 commits into
nightwatchjs:mainfrom
harshit-browserstack:sdk-5914-suppress-not-found-errors
May 15, 2026
Merged

fix(observability,session-status): don't mark suppressNotFoundErrors commands as test failures#53
shubhamkd merged 2 commits into
nightwatchjs:mainfrom
harshit-browserstack:sdk-5914-suppress-not-found-errors

Conversation

@harshit-browserstack
Copy link
Copy Markdown
Contributor

Fix: TestRunFinished and session status incorrectly marked failed for commands with suppressNotFoundErrors: true

Linked ticket: BrowserStack SDK-5914 / Freshdesk 1617613 (SmartVault)
Target package release: @nightwatch/browserstack@3.10.2

Summary

sendTestRunEvent('TestRunFinished', ...) flips test_run.result to "failed" whenever any command in the test envelope carries status:"fail", even when the command was a Nightwatch isVisible / isPresent invocation with suppressNotFoundErrors:true on an absent element. Nightwatch itself reports the testcase as status:"pass" with failed:0, the local runner exits clean, and BrowserStack Automate marks the session passed. Only Test Observability disagrees, producing a confusing pass/fail split for the same run.

Same defect shape lives in nightwatch/globals.js where the plugin sets the BrowserStack session status via browserstack_executor: setSessionStatus — folded into this PR so both surfaces stay consistent.

Root cause

Two sites read the per-command status array without consulting (a) the command's own args[0].suppressNotFoundErrors opt-out or (b) the testcase envelope's rollup (status, failed, errors):

src/testObservability.jsTestRunFinished branch in sendTestRunEvent:

const failedCommand = eventData.commands.find(cmd => cmd.status === 'fail');
if (failedCommand) { testData.result = 'failed'; ... }

nightwatch/globals.jssetSessionStatus path:

const failedCommand = eventData?.commands && Array.isArray(eventData.commands)
  ? eventData.commands.find(cmd => cmd.status === 'fail')
  : null;
const status = failedCommand ? 'failed' : 'passed';

A Nightwatch isVisible({suppressNotFoundErrors:true}) on an absent element internally records status:'fail' even though the caller opted out of failure semantics, so both sites flip to "failed" against the test's actual outcome.

Fix

  1. New helper helper.isSuppressedFailure(cmd) in src/utils/helper.js — returns true when a status:'fail' command's args[0] carries suppressNotFoundErrors:true. Handles both shapes Nightwatch emits: object literal and JSON-encoded string.
  2. src/testObservability.jssendTestRunEvent filters suppressed-failure commands out of the commands.find(...) call. Additionally, the testcase-envelope rollup (status:'pass' && failed:0 && errors:0) is consulted before flipping testData.result — if Nightwatch already says the test passed, the plugin trusts that rather than re-deriving from the raw command log.
  3. nightwatch/globals.jssetSessionStatus path uses the same helper and the same envelope-rollup guard, keeping the Automate session status aligned with TRA.

The behavior is additive: a non-suppressed status:'fail' command alongside an envelope rollup that says failed continues to mark the test failed exactly as before. Verified by the regression tests below.

Wire evidence

Captured against the customer's actual repro (a 2-test Nightwatch suite, one control + one conditional isVisible({suppressNotFoundErrors:true})) running with @nightwatch/browserstack@3.10.0. Same spec, same environment, only the plugin source differs.

Before:

TestRunFinished | Control                | result: "passed"
TestRunFinished | Conditional (suppress) | result: "failed",
                                          failure: [{backtrace: ["", ""]}],
                                          failure_reason: null

failure_reason: null and the empty backtrace are the smoking gun — there is no real failure to render; the plugin synthesized one from the suppressed isVisible command's status:'fail'.

After:

TestRunFinished | Control                | result: "passed"
TestRunFinished | Conditional (suppress) | result: "passed"

No failure, failure_reason, or failure_type fields. Control test unaffected (regression-negative).

Test plan

New unit-test file test/src/test-observability/sendTestRunEvent.js (4 cases — sendTestRunEvent had zero existing coverage, which is the structural reason this bug shipped):

  1. Passes when the only failing command opted into suppressNotFoundErrors:true and args[0] is an object literal.
  2. Passes when the only failing command opted into suppressNotFoundErrors:true and args[0] is a JSON-encoded string (the shape the customer's Nightwatch report uses).
  3. Still fails when a real assert.* failure is present — envelope rollup says failed:1, the patch must propagate the failure.
  4. Still fails when a non-suppressed command fails alongside a suppressed one — failedCommand must skip the suppressed entry and pick the real one; failure_type set correctly.

Full mocha suite: 55 passing (was 51).

Manual reproduction

The customer-attached reproduction repository runs against a real BrowserStack build:

npx nightwatch -c browserstack.conf.js -e chrome tests/observabilityBugRepro.js

With the customer's environment (BROWSERSTACK_USER / BROWSERSTACK_KEY, default Chrome on Windows 11), both tests land result:"passed" on the wire and on the TRA dashboard after upgrading to a build that includes this patch.

Compat note

  • No public API change. Helper is namespaced under src/utils/helper.js like the other utilities in the file.
  • No new dependencies.
  • No breaking change for tests with genuine failures.
  • The envelope-rollup guard is the more general of the two checks — even if Nightwatch adds a new opt-out flag in the future (e.g. silent:true, optional:true), as long as the testcase rollup correctly reports status:'pass', the plugin will stop overriding it.

…t failures

Nightwatch records every isVisible / isPresent timeout with status:'fail'
internally, including calls where the caller explicitly opted into "not found
is fine" via suppressNotFoundErrors:true. sendTestRunEvent and the
setSessionStatus path in globals.js both scanned commands[] for any
status:'fail' and flipped the test/session to "failed" without checking the
opt-out or the testcase envelope's pass/fail rollup, so tests Nightwatch and
Automate considered passed were reported as failed in Test Observability.

- Add helper.isSuppressedFailure(cmd) — handles args[0] as object or JSON string.
- sendTestRunEvent: filter suppressed-failure commands and trust the
  envelope rollup (status:'pass' && failed:0 && errors:0) over a stray
  command-level fail status.
- globals.js setSessionStatus: same fix, keeps Automate session status
  consistent with TRA.
- Add 4 regression tests for sendTestRunEvent (previously uncovered).

Linked: BrowserStack SDK-5914
…r coverage

- src/utils/helper.js: satisfy padding-line-between-statements; log a
  debug message when args[0] is an unparseable JSON string; clarify in
  the doc-comment why only args[0] is inspected.
- nightwatch/globals.js: clarify the eventData===null fallback behavior
  matches the pre-fix code (status defaults to 'passed').
- test/src/test-observability/sendTestRunEvent.js: tighten tests 3 & 4
  to lock failure_reason content and backtrace shape — exactly the wire
  fields the original bug malformed.
- test/src/utils/helper.js: add 10 unit tests for isSuppressedFailure
  covering null/undefined/primitive args[0], malformed JSON strings,
  object/string opt-out shapes, and falsy variants. Both call sites
  (sendTestRunEvent and setSessionStatus) are covered by reference.
@harshit-browserstack harshit-browserstack changed the title fix(observability): don't mark suppressNotFoundErrors commands as test failures fix(observability,session-status): don't mark suppressNotFoundErrors commands as test failures May 12, 2026
@shubhamkd shubhamkd merged commit 1a88995 into nightwatchjs:main May 15, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants