From 389d6a8b5e8cabced95b1a180b8ae294c18ae907 Mon Sep 17 00:00:00 2001 From: Mihir Rawool Date: Thu, 23 Jul 2026 06:06:29 +0530 Subject: [PATCH 1/3] LTS: populate duration_in_ms on Nightwatch TestRunFinished MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Main-path (non-cucumber, non-hook, non-skipped) Nightwatch TestRunFinished never carried duration_in_ms. Downstream is a straight passthrough (observability-pipeline EventProcessorService line 874 + 1325 in buildBtcerForLts), so BTCER.duration ended up NULL on every LTS Nightwatch row — and every duration-derived Tests-tab metric (min/avg/p50/p75/p90/p95/p99/max) rendered as 0. Fix gated on helper.isLoadTestingSession() so non-LTS emits stay byte-identical. Duration computed from the envelope timestamps (finished_at - started_at) that sendTestRunEvent already assigns for the TestRunFinished branch. Verified end-to-end: patched plugin + local Selenium jar + real Chrome + BROWSERSTACK_LTS_SESSION_ID set → TestRunFinished payload carries duration_in_ms = 145 (matches the timestamp delta). Contract check with LTS env off → duration_in_ms absent from payload. Two unit tests added to sendTestRunEvent.spec cover both paths. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/testObservability.js | 6 +++++ .../test-observability/sendTestRunEvent.js | 27 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/testObservability.js b/src/testObservability.js index 8113cb1..bb47752 100644 --- a/src/testObservability.js +++ b/src/testObservability.js @@ -535,6 +535,12 @@ class TestObservability { if (eventType === 'TestRunFinished') { const eventData = test.envelope[testName].testcase; testData.finished_at = eventData.endTimestamp ? new Date(eventData.endTimestamp).toISOString() : new Date(startTimestamp).toISOString(); + // LTS-only: main-path Nightwatch TestRunFinished doesn't carry duration_in_ms + // (cucumber/hook/skipped paths do). Compute from timestamps so BTCER duration + // populates for LTS builds. Non-LTS emits unchanged. + if (helper.isLoadTestingSession()) { + testData.duration_in_ms = new Date(testData.finished_at).getTime() - new Date(testData.started_at).getTime(); + } testData.result = 'passed'; if (eventData && eventData.commands && Array.isArray(eventData.commands)) { const failedCommand = eventData.commands.find(cmd => cmd.status === 'fail' && !helper.isSuppressedFailure(cmd)); diff --git a/test/src/test-observability/sendTestRunEvent.js b/test/src/test-observability/sendTestRunEvent.js index 8271507..ed32e50 100644 --- a/test/src/test-observability/sendTestRunEvent.js +++ b/test/src/test-observability/sendTestRunEvent.js @@ -161,4 +161,31 @@ describe('TestObservability - sendTestRunEvent (suppressNotFoundErrors)', functi `expected failure_reason to reference the real failure, got: ${this.uploaded.test_run.failure_reason}` ); }); + + // LTS regression: main-path Nightwatch TestRunFinished never carried + // duration_in_ms, leaving BTCER.duration NULL on load-testing builds and + // zeroing every duration-derived Tests-tab metric (min/avg/p50/p95/max). + // The fix computes duration from the envelope timestamps only when + // helper.isLoadTestingSession() is true. Non-LTS runs remain unchanged. + it('populates duration_in_ms from timestamps on LTS runs', async () => { + this.sandbox.stub(helper, 'isLoadTestingSession').returns(true); + const commands = [{name: 'url', args: ['https://example.com'], status: 'pass'}]; + + await this.testObservability.sendTestRunEvent('TestRunFinished', buildTest(commands), 'uuid-lts-1'); + + // Fixture: startTimestamp=1700000000000, endTimestamp=1700000001000 → 1000 ms. + assert.strictEqual(this.uploaded.test_run.duration_in_ms, 1000); + }); + + it('leaves duration_in_ms unset on non-LTS runs', async () => { + this.sandbox.stub(helper, 'isLoadTestingSession').returns(false); + const commands = [{name: 'url', args: ['https://example.com'], status: 'pass'}]; + + await this.testObservability.sendTestRunEvent('TestRunFinished', buildTest(commands), 'uuid-lts-2'); + + assert.ok( + !('duration_in_ms' in this.uploaded.test_run), + 'expected no duration_in_ms on non-LTS runs (contract preserved)' + ); + }); }); From c976a0bbe13a6d7870d1cf918d263ac5f25f71c7 Mon Sep 17 00:00:00 2001 From: Mihir Rawool Date: Mon, 27 Jul 2026 22:08:22 +0530 Subject: [PATCH 2/3] LTS: attach per-step data to meta.steps on Nightwatch TestRunFinished MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The LCNC compiler emits each recorded step wrapped in a bstackStep() helper that captures per-step timings and pushes them to globalThis.__bstack_steps. This change lets the plugin surface that buffer to Testhub so the mv_btcer_step_metrics_v5 MV can fan it out into btcer_step_metrics_v5 — the table Step Insights aggregates on across builds. Two touch points, both gated on helper.isLoadTestingSession() (same gate as the duration_in_ms fix, so non-LTS Automate customers see byte-identical uploads): - nightwatch/globals.js: reset globalThis.__bstack_steps on every TestRunStarted so retries and per-VU iterations get clean buffers. - src/testObservability.js: attach the buffer to testData.meta.steps on TestRunFinished. Non-destructive merge — preserves whatever meta fields (e.g. configuration_id) the LT pipeline had already written. Verified end-to-end against real Testhub: - Compiled a sample LCNC-style Nightwatch spec (tests/lcnc_mimic_test.js in local mimic) with 6 bstackStep-wrapped steps. - Ran through local chromedriver with BROWSERSTACK_LTS_SESSION_ID set → build creation succeeded, TestRunFinished payload carried test_run.meta.steps as a 6-entry array. - ClickHouse: build_test_case_execution_results_v5 row landed with meta.steps present, mv_btcer_step_metrics_v5 fanned out 6 rows into btcer_step_metrics_v5 with correct step_text, step_result and step_duration values (846, 597, 118, 72, 47, 24 ms). Three unit tests added to sendTestRunEvent.spec cover: attach on LTS, omit on non-LTS, and empty/missing buffer. Co-Authored-By: Claude Opus 4.7 --- nightwatch/globals.js | 4 ++ src/testObservability.js | 7 +++ .../test-observability/sendTestRunEvent.js | 47 +++++++++++++++++++ 3 files changed, 58 insertions(+) diff --git a/nightwatch/globals.js b/nightwatch/globals.js index 9476760..c2413e8 100644 --- a/nightwatch/globals.js +++ b/nightwatch/globals.js @@ -302,6 +302,10 @@ module.exports = { if (testRunner !== 'cucumber'){ const uuid = TestMap.storeTestDetails(test); process.env.TEST_RUN_UUID = uuid; + // LTS-only: reset the per-test step buffer that bstackStep() (injected + // by the LCNC compiler into the compiled spec) writes into. Read back + // in testObservability.sendTestRunEvent at TestRunFinished. + if (helper.isLoadTestingSession()) {globalThis.__bstack_steps = [];} // Capture the live session id at TestRunStarted (when browser.end() // hasn't run yet). TestRunFinished's async handler can race with // afterEach: by the time it executes, browser.sessionId may already diff --git a/src/testObservability.js b/src/testObservability.js index bb47752..9775c4c 100644 --- a/src/testObservability.js +++ b/src/testObservability.js @@ -540,6 +540,13 @@ class TestObservability { // populates for LTS builds. Non-LTS emits unchanged. if (helper.isLoadTestingSession()) { testData.duration_in_ms = new Date(testData.finished_at).getTime() - new Date(testData.started_at).getTime(); + // LTS-only: attach per-step data collected by bstackStep() (injected by + // the LCNC compiler into the compiled spec). Empty/absent → skip so + // non-instrumented specs stay backward-compatible. + const steps = globalThis.__bstack_steps; + if (Array.isArray(steps) && steps.length) { + testData.meta = {...(testData.meta || {}), steps}; + } } testData.result = 'passed'; if (eventData && eventData.commands && Array.isArray(eventData.commands)) { diff --git a/test/src/test-observability/sendTestRunEvent.js b/test/src/test-observability/sendTestRunEvent.js index ed32e50..1ac07f0 100644 --- a/test/src/test-observability/sendTestRunEvent.js +++ b/test/src/test-observability/sendTestRunEvent.js @@ -188,4 +188,51 @@ describe('TestObservability - sendTestRunEvent (suppressNotFoundErrors)', functi 'expected no duration_in_ms on non-LTS runs (contract preserved)' ); }); + + // LTS step-level insights: the LCNC compiler wraps each recorded step in a + // bstackStep() helper that pushes {id, text, duration, ...} to + // globalThis.__bstack_steps. sendTestRunEvent must attach that array to + // testData.meta.steps at TestRunFinished (only when isLoadTestingSession() + // is true) so the Testhub MV mv_btcer_step_metrics_v5 fans it out into the + // btcer_step_metrics_v5 table that Step Insights aggregates on. + it('attaches globalThis.__bstack_steps to meta.steps on LTS runs', async () => { + this.sandbox.stub(helper, 'isLoadTestingSession').returns(true); + const steps = [ + {id: 'a', text: 'Open page', keyword: '', duration: 800, started_at: '2026-07-27T16:00:00.000', finished_at: '2026-07-27T16:00:00.800', result: 'passed', failure: null}, + {id: 'b', text: 'Click X', keyword: '', duration: 120, started_at: '2026-07-27T16:00:00.800', finished_at: '2026-07-27T16:00:00.920', result: 'passed', failure: null} + ]; + globalThis.__bstack_steps = steps; + const commands = [{name: 'url', args: ['https://example.com'], status: 'pass'}]; + + try { + await this.testObservability.sendTestRunEvent('TestRunFinished', buildTest(commands), 'uuid-lts-steps-1'); + assert.deepStrictEqual(this.uploaded.test_run.meta && this.uploaded.test_run.meta.steps, steps); + } finally { + delete globalThis.__bstack_steps; + } + }); + + it('omits meta.steps on non-LTS runs even when the buffer is populated', async () => { + this.sandbox.stub(helper, 'isLoadTestingSession').returns(false); + globalThis.__bstack_steps = [{id: 'a', text: 'Open page', duration: 800, result: 'passed'}]; + const commands = [{name: 'url', args: ['https://example.com'], status: 'pass'}]; + + try { + await this.testObservability.sendTestRunEvent('TestRunFinished', buildTest(commands), 'uuid-lts-steps-2'); + const meta = this.uploaded.test_run.meta; + assert.ok(!meta || !('steps' in meta), 'expected no meta.steps on non-LTS runs (contract preserved)'); + } finally { + delete globalThis.__bstack_steps; + } + }); + + it('leaves meta.steps unset on LTS runs when the buffer is empty or missing', async () => { + this.sandbox.stub(helper, 'isLoadTestingSession').returns(true); + delete globalThis.__bstack_steps; + const commands = [{name: 'url', args: ['https://example.com'], status: 'pass'}]; + + await this.testObservability.sendTestRunEvent('TestRunFinished', buildTest(commands), 'uuid-lts-steps-3'); + const meta = this.uploaded.test_run.meta; + assert.ok(!meta || !('steps' in meta), 'expected no meta.steps when nothing was recorded'); + }); }); From 5dea3eadb116efb97802280e51044a8404eecf7d Mon Sep 17 00:00:00 2001 From: Mihir Rawool Date: Thu, 30 Jul 2026 05:25:30 +0530 Subject: [PATCH 3/3] LTS: use global instead of globalThis for eslint env compat The repo's eslint config doesn't declare es2020 env, so globalThis trips no-undef. `global` is Node's builtin and works in every runtime this plugin actually ships to. Also splits the one-line if-block so the `semi` rule's omitLastInOneLineBlock stops flagging the assignment's own terminator. --- nightwatch/globals.js | 4 +++- src/testObservability.js | 2 +- test/src/test-observability/sendTestRunEvent.js | 14 +++++++------- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/nightwatch/globals.js b/nightwatch/globals.js index c2413e8..7eb160f 100644 --- a/nightwatch/globals.js +++ b/nightwatch/globals.js @@ -305,7 +305,9 @@ module.exports = { // LTS-only: reset the per-test step buffer that bstackStep() (injected // by the LCNC compiler into the compiled spec) writes into. Read back // in testObservability.sendTestRunEvent at TestRunFinished. - if (helper.isLoadTestingSession()) {globalThis.__bstack_steps = [];} + if (helper.isLoadTestingSession()) { + global.__bstack_steps = []; + } // Capture the live session id at TestRunStarted (when browser.end() // hasn't run yet). TestRunFinished's async handler can race with // afterEach: by the time it executes, browser.sessionId may already diff --git a/src/testObservability.js b/src/testObservability.js index 9775c4c..f2f70dc 100644 --- a/src/testObservability.js +++ b/src/testObservability.js @@ -543,7 +543,7 @@ class TestObservability { // LTS-only: attach per-step data collected by bstackStep() (injected by // the LCNC compiler into the compiled spec). Empty/absent → skip so // non-instrumented specs stay backward-compatible. - const steps = globalThis.__bstack_steps; + const steps = global.__bstack_steps; if (Array.isArray(steps) && steps.length) { testData.meta = {...(testData.meta || {}), steps}; } diff --git a/test/src/test-observability/sendTestRunEvent.js b/test/src/test-observability/sendTestRunEvent.js index 1ac07f0..6dfa7b2 100644 --- a/test/src/test-observability/sendTestRunEvent.js +++ b/test/src/test-observability/sendTestRunEvent.js @@ -191,30 +191,30 @@ describe('TestObservability - sendTestRunEvent (suppressNotFoundErrors)', functi // LTS step-level insights: the LCNC compiler wraps each recorded step in a // bstackStep() helper that pushes {id, text, duration, ...} to - // globalThis.__bstack_steps. sendTestRunEvent must attach that array to + // global.__bstack_steps. sendTestRunEvent must attach that array to // testData.meta.steps at TestRunFinished (only when isLoadTestingSession() // is true) so the Testhub MV mv_btcer_step_metrics_v5 fans it out into the // btcer_step_metrics_v5 table that Step Insights aggregates on. - it('attaches globalThis.__bstack_steps to meta.steps on LTS runs', async () => { + it('attaches global.__bstack_steps to meta.steps on LTS runs', async () => { this.sandbox.stub(helper, 'isLoadTestingSession').returns(true); const steps = [ {id: 'a', text: 'Open page', keyword: '', duration: 800, started_at: '2026-07-27T16:00:00.000', finished_at: '2026-07-27T16:00:00.800', result: 'passed', failure: null}, {id: 'b', text: 'Click X', keyword: '', duration: 120, started_at: '2026-07-27T16:00:00.800', finished_at: '2026-07-27T16:00:00.920', result: 'passed', failure: null} ]; - globalThis.__bstack_steps = steps; + global.__bstack_steps = steps; const commands = [{name: 'url', args: ['https://example.com'], status: 'pass'}]; try { await this.testObservability.sendTestRunEvent('TestRunFinished', buildTest(commands), 'uuid-lts-steps-1'); assert.deepStrictEqual(this.uploaded.test_run.meta && this.uploaded.test_run.meta.steps, steps); } finally { - delete globalThis.__bstack_steps; + delete global.__bstack_steps; } }); it('omits meta.steps on non-LTS runs even when the buffer is populated', async () => { this.sandbox.stub(helper, 'isLoadTestingSession').returns(false); - globalThis.__bstack_steps = [{id: 'a', text: 'Open page', duration: 800, result: 'passed'}]; + global.__bstack_steps = [{id: 'a', text: 'Open page', duration: 800, result: 'passed'}]; const commands = [{name: 'url', args: ['https://example.com'], status: 'pass'}]; try { @@ -222,13 +222,13 @@ describe('TestObservability - sendTestRunEvent (suppressNotFoundErrors)', functi const meta = this.uploaded.test_run.meta; assert.ok(!meta || !('steps' in meta), 'expected no meta.steps on non-LTS runs (contract preserved)'); } finally { - delete globalThis.__bstack_steps; + delete global.__bstack_steps; } }); it('leaves meta.steps unset on LTS runs when the buffer is empty or missing', async () => { this.sandbox.stub(helper, 'isLoadTestingSession').returns(true); - delete globalThis.__bstack_steps; + delete global.__bstack_steps; const commands = [{name: 'url', args: ['https://example.com'], status: 'pass'}]; await this.testObservability.sendTestRunEvent('TestRunFinished', buildTest(commands), 'uuid-lts-steps-3');