fix(observability): lint, tighten regression tests, and broaden helper 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.

Author: harshit-browserstack

nightwatch/globals.js

@@ -310,6 +310,9 @@ module.exports = {
             // Skip commands the caller opted out of via suppressNotFoundErrors:true
             // and trust Nightwatch's envelope-level rollup over a stray command-level
             // fail status. Same defect shape as testObservability.js sendTestRunEvent.
+            // When eventData itself is missing (session aborted before TestRunStarted
+            // populated the envelope), failedCommand stays null and status defaults
+            // to 'passed' — matches the pre-fix behaviour for that edge case.
             const failedCommand = eventData?.commands && Array.isArray(eventData.commands)
               ? eventData.commands.find(cmd => cmd.status === 'fail' && !helper.isSuppressedFailure(cmd))
               : null;

src/utils/helper.js

@@ -65,15 +65,24 @@ exports.isObject = value => (!this.isUndefined(value) && value.constructor === O
 // A Nightwatch command is recorded with status:'fail' even when the caller
 // explicitly opted into "not found is fine" via `suppressNotFoundErrors:true`.
 // Those commands carry no failure semantics for the test and must not flip
-// test/session status. `args[0]` is the command options object — Nightwatch
-// serializes it to a JSON string in some reporter paths, so accept both shapes.
+// test/session status. Only `args[0]` is inspected — Nightwatch's reporter
+// always serializes element-command options as the first positional argument,
+// and either as an object literal or as a JSON-encoded string depending on
+// the reporter path. Both shapes must be accepted.
 exports.isSuppressedFailure = (cmd) => {
   if (!cmd || cmd.status !== 'fail' || !Array.isArray(cmd.args) || cmd.args.length === 0) {return false}
   const first = cmd.args[0];
   let opts = first;
   if (typeof first === 'string') {
-    try {opts = JSON.parse(first)} catch (e) {return false}
+    try {
+      opts = JSON.parse(first);
+    } catch (e) {
+      Logger.debug(`isSuppressedFailure: could not parse args[0] for cmd ${cmd.name || '<unnamed>'}: ${e.message}`);
+
+      return false;
+    }
   }
+
   return !!(opts && typeof opts === 'object' && opts.suppressNotFoundErrors === true);
 };
 

test/src/test-observability/sendTestRunEvent.js

@@ -118,6 +118,13 @@ describe('TestObservability - sendTestRunEvent (suppressNotFoundErrors)', functi
 
     assert.strictEqual(this.uploaded.test_run.result, 'failed');
     assert.strictEqual(this.uploaded.test_run.failure_type, 'AssertionError');
+    // Lock the wire-shape that the original bug malformed (failure_reason:null,
+    // backtrace:["",""]). The patch must propagate the real failure detail.
+    assert.strictEqual(this.uploaded.test_run.failure_reason, 'Expected title to contain "Google"');
+    assert.deepStrictEqual(
+      this.uploaded.test_run.failure[0].backtrace,
+      ['Expected title to contain "Google"', 'AssertionError']
+    );
   });
 
   it('still marks the test failed when a non-suppressed command failed alongside a suppressed one', async () => {
@@ -145,7 +152,13 @@ describe('TestObservability - sendTestRunEvent (suppressNotFoundErrors)', functi
     );
 
     assert.strictEqual(this.uploaded.test_run.result, 'failed');
-    // failedCommand must skip the suppressed one and pick the real one.
+    // failedCommand must skip the suppressed one and pick the real one — confirm
+    // by asserting the failure_reason references the mandatory selector, not the
+    // suppressed optional one.
     assert.strictEqual(this.uploaded.test_run.failure_type, 'UnhandledError');
+    assert.ok(
+      this.uploaded.test_run.failure_reason.includes('#mandatory'),
+      `expected failure_reason to reference the real failure, got: ${this.uploaded.test_run.failure_reason}`
+    );
   });
 });

test/src/utils/helper.js

@@ -353,3 +353,69 @@ describe('isBrowserstackInfra', () => {
   });
 
 });
+
+describe('isSuppressedFailure', () => {
+  // Shared primitive used by both src/testObservability.js (sendTestRunEvent)
+  // and nightwatch/globals.js (setSessionStatus). Covers both call sites by
+  // reference — any regression here breaks both surfaces equivalently.
+  let isSuppressedFailure;
+  before(() => {
+    isSuppressedFailure = require('../../../src/utils/helper').isSuppressedFailure;
+  });
+
+  it('returns false for null / undefined commands', () => {
+    expect(isSuppressedFailure(null)).to.be.false;
+    expect(isSuppressedFailure(undefined)).to.be.false;
+  });
+
+  it('returns false for commands that did not fail', () => {
+    expect(isSuppressedFailure({status: 'pass', args: [{suppressNotFoundErrors: true}]})).to.be.false;
+  });
+
+  it('returns false when args is missing or empty', () => {
+    expect(isSuppressedFailure({status: 'fail'})).to.be.false;
+    expect(isSuppressedFailure({status: 'fail', args: []})).to.be.false;
+  });
+
+  it('returns false when args is not an array', () => {
+    expect(isSuppressedFailure({status: 'fail', args: {suppressNotFoundErrors: true}})).to.be.false;
+    expect(isSuppressedFailure({status: 'fail', args: 'not-an-array'})).to.be.false;
+  });
+
+  it('returns false when args[0] is a primitive', () => {
+    expect(isSuppressedFailure({status: 'fail', args: [null]})).to.be.false;
+    expect(isSuppressedFailure({status: 'fail', args: [42]})).to.be.false;
+    expect(isSuppressedFailure({status: 'fail', args: [true]})).to.be.false;
+  });
+
+  it('returns true when args[0] is an object with suppressNotFoundErrors:true', () => {
+    expect(isSuppressedFailure({
+      status: 'fail',
+      args: [{selector: '#x', suppressNotFoundErrors: true, timeout: 2000}, null]
+    })).to.be.true;
+  });
+
+  it('returns true when args[0] is a JSON string carrying suppressNotFoundErrors:true', () => {
+    expect(isSuppressedFailure({
+      status: 'fail',
+      args: ['{"selector":"#x","suppressNotFoundErrors":true,"timeout":2000}', null]
+    })).to.be.true;
+  });
+
+  it('returns false when args[0] is a malformed JSON string (safe default)', () => {
+    // Unparseable strings default to "not suppressed" — i.e., the test stays
+    // failed. That's the safer direction, matches the rest of the guard's
+    // bias to under-suppress rather than over-suppress.
+    expect(isSuppressedFailure({status: 'fail', args: ['{this is not json']})).to.be.false;
+  });
+
+  it('returns false when args[0] is an object without suppressNotFoundErrors', () => {
+    expect(isSuppressedFailure({status: 'fail', args: [{selector: '#x', timeout: 2000}]})).to.be.false;
+  });
+
+  it('returns false when suppressNotFoundErrors is falsy', () => {
+    expect(isSuppressedFailure({status: 'fail', args: [{suppressNotFoundErrors: false}]})).to.be.false;
+    expect(isSuppressedFailure({status: 'fail', args: [{suppressNotFoundErrors: 'true'}]})).to.be.false;
+  });
+
+});