From ed3a58aba7d6ef3868bffe6c9b4e4dd698f8c55f Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Wed, 9 Sep 2026 19:26:11 +0530 Subject: [PATCH 1/3] feat(o11y): stamp testhubBuildUuid + buildProductMap on cypress build caps [SDK-7285] Cypress is the only SDK line that stamps no TestHub capability at all: 0 of 16,629 sessions carry one across all 32 CLI versions. That leaves it with a single route into Test Reporting & Analytics -- the BTCER session_hashed_id fallback -- and that misses ~32.5% of its sessions, which are then counted against ingestion stability with no build to attach to (2,917 sessions / 0.5882pp in the measured cohort, across 76 accounts). caps() already runs after build start, so the uuid and the product map are both in hand; getProductMap() is reused rather than reimplemented, and evaluating it here reports the products still enabled after the accessibility/observability gates rather than the ones requested. Both keys are written unconditionally. The metric treats an absent key and an empty string identically, so the empty string costs nothing and preserves the distinction between a cap writer that ran with no build to name and no writer running at all -- which for cypress is currently unobservable. Verified end-to-end on production: the build-create POST carries testhubBuildUuid=xcsouiygyettrxqdxyxmi18qnkyb6alfxqnj8x4z, matching the TestHub build the run created. Co-Authored-By: Claude Opus 5 --- bin/helpers/capabilityHelper.js | 11 +++- test/unit/bin/helpers/capabilityHelper.js | 65 +++++++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/bin/helpers/capabilityHelper.js b/bin/helpers/capabilityHelper.js index f380b981..078233f4 100644 --- a/bin/helpers/capabilityHelper.js +++ b/bin/helpers/capabilityHelper.js @@ -4,7 +4,8 @@ const { readCypressConfigFile } = require('./readCypressConfigUtil'); const logger = require("./logger").winstonLogger, Constants = require("./constants"), - Utils = require("./utils"); + Utils = require("./utils"), + testhubUtils = require("../testhub/utils"); const caps = (bsConfig, zip) => { return new Promise(function (resolve, reject) { @@ -131,6 +132,14 @@ const caps = (bsConfig, zip) => { obj.run_settings = JSON.stringify(bsConfig.run_settings); } + // The only route by which a cypress session can name its TestHub build: every session + // this build spawns inherits these caps. Written unconditionally so an empty uuid records + // that build start ran and had nothing to name, which an absent key cannot express. + obj.testhubBuildUuid = process.env.BROWSERSTACK_TESTHUB_UUID || ""; + obj.buildProductMap = testhubUtils.getProductMap(bsConfig); + + logger.debug(`TestHub build uuid stamped on caps: ${obj.testhubBuildUuid || ""}`); + obj.cypress_cli_user_agent = Utils.getUserAgent(); logger.info(`Cypress CLI User Agent: ${obj.cypress_cli_user_agent}`); diff --git a/test/unit/bin/helpers/capabilityHelper.js b/test/unit/bin/helpers/capabilityHelper.js index 490d3dee..95784631 100644 --- a/test/unit/bin/helpers/capabilityHelper.js +++ b/test/unit/bin/helpers/capabilityHelper.js @@ -5,6 +5,7 @@ const chai = require("chai"), const capabilityHelper = require("../../../../bin/helpers/capabilityHelper"), Constants = require("../../../../bin/helpers/constants"), + testhubUtils = require("../../../../bin/testhub/utils"), logger = require("../../../../bin/helpers/logger").winstonLogger; chai.use(chaiAsPromised); @@ -562,6 +563,70 @@ describe("capabilityHelper.js", () => { }); }); }); + + context("testhub build attribution", () => { + const bsConfig = { + auth: { + username: "random", + access_key: "random", + }, + browsers: [ + { + browser: "chrome", + os: "Windows 10", + versions: ["78"], + }, + ], + run_settings: {}, + }; + const productMap = { + observability: true, + accessibility: false, + percy: false, + automate: true, + app_automate: false, + }; + let productMapStub; + let originalTesthubUuid; + + beforeEach(() => { + originalTesthubUuid = process.env.BROWSERSTACK_TESTHUB_UUID; + productMapStub = sinon.stub(testhubUtils, "getProductMap").returns(productMap); + }); + + afterEach(() => { + productMapStub.restore(); + if (originalTesthubUuid === undefined) { + delete process.env.BROWSERSTACK_TESTHUB_UUID; + } else { + process.env.BROWSERSTACK_TESTHUB_UUID = originalTesthubUuid; + } + }); + + it("stamps the testhub build uuid and the product map on the caps", () => { + process.env.BROWSERSTACK_TESTHUB_UUID = "some-testhub-build-uuid"; + return capabilityHelper + .caps(bsConfig, { zip_url: "bs://" }) + .then(function (data) { + let parsed_data = JSON.parse(data); + chai.assert.equal(parsed_data.testhubBuildUuid, "some-testhub-build-uuid"); + chai.assert.deepEqual(parsed_data.buildProductMap, productMap); + sinon.assert.calledWith(productMapStub, bsConfig); + }); + }); + + it("stamps an empty testhub build uuid when build start produced none", () => { + delete process.env.BROWSERSTACK_TESTHUB_UUID; + return capabilityHelper + .caps(bsConfig, { zip_url: "bs://" }) + .then(function (data) { + let parsed_data = JSON.parse(data); + chai.assert.equal(parsed_data.testhubBuildUuid, ""); + chai.assert.isTrue(Object.prototype.hasOwnProperty.call(parsed_data, "testhubBuildUuid")); + chai.assert.deepEqual(parsed_data.buildProductMap, productMap); + }); + }); + }); }); describe("addCypressZipStartLocation", () => { From 5c9a9e07f61841c7e1c79484630d8f11473bef05 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Thu, 10 Sep 2026 12:37:55 +0530 Subject: [PATCH 2/3] fix(o11y): buildProductMap must report observability off when there is no build [SDK-7285] The product map is stamped twice with different meanings: on the build-start request it is INTENT (what the run asked for), and on the session caps it is OUTCOME (what is actually live). Only the first was true. extractDataFromResponse already clears BROWSERSTACK_TEST_OBSERVABILITY when a 2xx comes back carrying observability.success=false, but launchBuild's catch -- a non-2xx or a network error, so the whole build-start-failed population -- only logged. The flag stayed true, so the session reported observability enabled with no build behind it. Verified on production against an injected collector 400: buildProductMap.observability was true before this change and is false after, same config, same fault. Routing that catch through handleErrorForObservability fixes it in one place and makes the failure paths consistent. That funnel writes the string "null" into BROWSERSTACK_TESTHUB_UUID, which the caps must not stamp as if it were a real uuid -- so the two changes land together. An explicit testObservability: false in config already resolved correctly through setTestObservabilityFlags; that needed no change and is now covered by a test so it cannot regress silently. Also: logBuildError treated only undefined as "no error object", so a null error logged the string "null" instead of the intended message. Affected the accessibility no-response path too. Co-Authored-By: Claude Opus 5 --- bin/helpers/capabilityHelper.js | 6 +- bin/testhub/testhubHandler.js | 4 +- bin/testhub/utils.js | 6 +- test/unit/bin/helpers/capabilityHelper.js | 82 ++++++++++++++++ test/unit/bin/testhub/utils.js | 111 ++++++++++++++++++++++ 5 files changed, 205 insertions(+), 4 deletions(-) create mode 100644 test/unit/bin/testhub/utils.js diff --git a/bin/helpers/capabilityHelper.js b/bin/helpers/capabilityHelper.js index 078233f4..7f8bef59 100644 --- a/bin/helpers/capabilityHelper.js +++ b/bin/helpers/capabilityHelper.js @@ -135,7 +135,11 @@ const caps = (bsConfig, zip) => { // The only route by which a cypress session can name its TestHub build: every session // this build spawns inherits these caps. Written unconditionally so an empty uuid records // that build start ran and had nothing to name, which an absent key cannot express. - obj.testhubBuildUuid = process.env.BROWSERSTACK_TESTHUB_UUID || ""; + // "null" is the sentinel a failed build start leaves behind, not a uuid. + const testhubBuildUuid = process.env.BROWSERSTACK_TESTHUB_UUID; + obj.testhubBuildUuid = Utils.isUndefined(testhubBuildUuid) || testhubBuildUuid === "null" + ? "" + : testhubBuildUuid; obj.buildProductMap = testhubUtils.getProductMap(bsConfig); logger.debug(`TestHub build uuid stamped on caps: ${obj.testhubBuildUuid || ""}`); diff --git a/bin/testhub/testhubHandler.js b/bin/testhub/testhubHandler.js index a6b4724a..e9cd1784 100644 --- a/bin/testhub/testhubHandler.js +++ b/bin/testhub/testhubHandler.js @@ -41,9 +41,9 @@ class TestHubHandler { const response = await nodeRequest( "POST", TESTHUB_CONSTANTS.TESTHUB_BUILD_API, data, config); const launchData = this.extractDataFromResponse(user_config, data, response, config); } catch (error) { - console.log(error); + logger.debug(`EXCEPTION IN BUILD START EVENT : ${error}`); + testhubUtils.handleErrorForObservability(error.success === false ? error : null); if (error.success === false) { // non 200 response - testhubUtils.logBuildError(error); return; } diff --git a/bin/testhub/utils.js b/bin/testhub/utils.js index 718bb595..aaca9b68 100644 --- a/bin/testhub/utils.js +++ b/bin/testhub/utils.js @@ -88,6 +88,10 @@ exports.setTestObservabilityVariables = ( }; exports.handleErrorForObservability = (error = null) => { + // Downstream reads isTestObservabilitySession(), not these ids, to decide whether + // observability is live. extractDataFromResponse clears it inline for a 2xx carrying + // success=false; this covers the paths that never get a usable response at all. + process.env.BROWSERSTACK_TEST_OBSERVABILITY = "false"; process.env.BROWSERSTACK_TESTHUB_UUID = "null"; process.env.BROWSERSTACK_TESTHUB_JWT = "null"; process.env.BS_TESTOPS_BUILD_COMPLETED = "false"; @@ -164,7 +168,7 @@ exports.handleErrorForAccessibility = (user_config, error = null) => { }; exports.logBuildError = (error, product = "") => { - if (error === undefined) { + if (isUndefined(error)) { logger.error(`${product.toUpperCase()} Build creation failed`); return; diff --git a/test/unit/bin/helpers/capabilityHelper.js b/test/unit/bin/helpers/capabilityHelper.js index 95784631..2cfc5699 100644 --- a/test/unit/bin/helpers/capabilityHelper.js +++ b/test/unit/bin/helpers/capabilityHelper.js @@ -6,6 +6,7 @@ const chai = require("chai"), const capabilityHelper = require("../../../../bin/helpers/capabilityHelper"), Constants = require("../../../../bin/helpers/constants"), testhubUtils = require("../../../../bin/testhub/utils"), + o11yHelper = require("../../../../bin/testObservability/helper/helper"), logger = require("../../../../bin/helpers/logger").winstonLogger; chai.use(chaiAsPromised); @@ -627,6 +628,87 @@ describe("capabilityHelper.js", () => { }); }); }); + + // These exercise the REAL getProductMap rather than a stub, because the thing under test is + // what the map SAYS, not that it is attached. + context("testhub build attribution — product map reflects reality", () => { + const ENV = ["BROWSERSTACK_TEST_OBSERVABILITY", "BROWSERSTACK_TESTHUB_UUID", + "BROWSERSTACK_TEST_ACCESSIBILITY", "BROWSERSTACK_AUTOMATION"]; + let saved; + + const bsConfigFor = (testObservability) => ({ + auth: { username: "random", access_key: "random" }, + browsers: [{ browser: "chrome", os: "Windows 10", versions: ["78"] }], + run_settings: { cypress_config_file: "./cypress.config.js" }, + testObservability, + }); + + beforeEach(() => { + saved = {}; + ENV.forEach((k) => { saved[k] = process.env[k]; }); + delete process.env.BROWSERSTACK_TEST_OBSERVABILITY; + process.env.BROWSERSTACK_TEST_ACCESSIBILITY = "false"; + process.env.BROWSERSTACK_AUTOMATION = "true"; + }); + + afterEach(() => { + ENV.forEach((k) => { + if (saved[k] === undefined) delete process.env[k]; + else process.env[k] = saved[k]; + }); + }); + + it("carries observability:false when the user explicitly disabled it in config", () => { + const bsConfig = bsConfigFor(false); + o11yHelper.setTestObservabilityFlags(bsConfig); + chai.assert.equal(process.env.BROWSERSTACK_TEST_OBSERVABILITY, "false", "precondition"); + + return capabilityHelper + .caps(bsConfig, { zip_url: "bs://" }) + .then(function (data) { + const parsed_data = JSON.parse(data); + chai.assert.isFalse(parsed_data.buildProductMap.observability); + chai.assert.equal(parsed_data.testhubBuildUuid, ""); + }); + }); + + it("carries observability:true when the user asked for it and build start succeeded", () => { + const bsConfig = bsConfigFor(true); + o11yHelper.setTestObservabilityFlags(bsConfig); + process.env.BROWSERSTACK_TESTHUB_UUID = "a-real-build-uuid"; + + return capabilityHelper + .caps(bsConfig, { zip_url: "bs://" }) + .then(function (data) { + const parsed_data = JSON.parse(data); + chai.assert.isTrue(parsed_data.buildProductMap.observability); + chai.assert.equal(parsed_data.testhubBuildUuid, "a-real-build-uuid"); + }); + }); + + it("flips observability to false and drops the null sentinel when build start failed", () => { + const bsConfig = bsConfigFor(true); + o11yHelper.setTestObservabilityFlags(bsConfig); + chai.assert.equal(process.env.BROWSERSTACK_TEST_OBSERVABILITY, "true", "precondition"); + + const errorStub = sinon.stub(logger, "error"); + try { + testhubUtils.handleErrorForObservability(); + } finally { + errorStub.restore(); + } + chai.assert.equal(process.env.BROWSERSTACK_TESTHUB_UUID, "null", "sentinel is what we guard against"); + + return capabilityHelper + .caps(bsConfig, { zip_url: "bs://" }) + .then(function (data) { + const parsed_data = JSON.parse(data); + chai.assert.isFalse(parsed_data.buildProductMap.observability); + chai.assert.equal(parsed_data.testhubBuildUuid, "", + 'the "null" sentinel must never be stamped as a uuid'); + }); + }); + }); }); describe("addCypressZipStartLocation", () => { diff --git a/test/unit/bin/testhub/utils.js b/test/unit/bin/testhub/utils.js new file mode 100644 index 00000000..5cc7eeee --- /dev/null +++ b/test/unit/bin/testhub/utils.js @@ -0,0 +1,111 @@ +const chai = require("chai"), + sinon = require("sinon"); + +const testhubUtils = require("../../../../bin/testhub/utils"), + logger = require("../../../../bin/helpers/logger").winstonLogger; + +describe("testhub/utils.js", () => { + const OBSERVABILITY_ENV = [ + "BROWSERSTACK_TEST_OBSERVABILITY", + "BROWSERSTACK_TESTHUB_UUID", + "BROWSERSTACK_TESTHUB_JWT", + "BS_TESTOPS_BUILD_COMPLETED", + "BS_TESTOPS_JWT", + "BS_TESTOPS_BUILD_HASHED_ID", + "BS_TESTOPS_ALLOW_SCREENSHOTS", + "BROWSERSTACK_TEST_ACCESSIBILITY", + "BROWSERSTACK_AUTOMATION", + ]; + let saved; + + beforeEach(() => { + saved = {}; + OBSERVABILITY_ENV.forEach((k) => { saved[k] = process.env[k]; }); + }); + + afterEach(() => { + OBSERVABILITY_ENV.forEach((k) => { + if (saved[k] === undefined) delete process.env[k]; + else process.env[k] = saved[k]; + }); + }); + + describe("getProductMap", () => { + it("reports observability false when the flag is not set to true", () => { + process.env.BROWSERSTACK_TEST_OBSERVABILITY = "false"; + process.env.BROWSERSTACK_TEST_ACCESSIBILITY = "false"; + process.env.BROWSERSTACK_AUTOMATION = "true"; + + chai.assert.deepEqual(testhubUtils.getProductMap({}), { + observability: false, + accessibility: false, + percy: false, + automate: true, + app_automate: false, + }); + }); + + it("reports observability true only while the flag says so", () => { + process.env.BROWSERSTACK_TEST_OBSERVABILITY = "true"; + process.env.BROWSERSTACK_TEST_ACCESSIBILITY = "false"; + process.env.BROWSERSTACK_AUTOMATION = "true"; + + chai.assert.isTrue(testhubUtils.getProductMap({}).observability); + }); + }); + + describe("handleErrorForObservability", () => { + let errorStub; + + beforeEach(() => { errorStub = sinon.stub(logger, "error"); }); + afterEach(() => { errorStub.restore(); }); + + it("turns observability off in the product map when build start fails", () => { + process.env.BROWSERSTACK_TEST_OBSERVABILITY = "true"; + process.env.BROWSERSTACK_TEST_ACCESSIBILITY = "false"; + process.env.BROWSERSTACK_AUTOMATION = "true"; + chai.assert.isTrue(testhubUtils.getProductMap({}).observability, "precondition"); + + testhubUtils.handleErrorForObservability(); + + chai.assert.equal(process.env.BROWSERSTACK_TEST_OBSERVABILITY, "false"); + chai.assert.isFalse(testhubUtils.getProductMap({}).observability); + }); + + it("leaves the uuid as the null sentinel and marks the build not completed", () => { + process.env.BROWSERSTACK_TESTHUB_UUID = "some-uuid"; + process.env.BS_TESTOPS_BUILD_COMPLETED = "true"; + + testhubUtils.handleErrorForObservability(); + + chai.assert.equal(process.env.BROWSERSTACK_TESTHUB_UUID, "null"); + chai.assert.equal(process.env.BS_TESTOPS_BUILD_COMPLETED, "false"); + }); + + it("does not report observability as still enabled to shouldProcessEventForTesthub", () => { + process.env.BROWSERSTACK_TEST_OBSERVABILITY = "true"; + process.env.BROWSERSTACK_TEST_ACCESSIBILITY = "false"; + + testhubUtils.handleErrorForObservability(); + + chai.assert.isFalse(testhubUtils.shouldProcessEventForTesthub()); + }); + }); + + describe("logBuildError", () => { + let errorStub; + + beforeEach(() => { errorStub = sinon.stub(logger, "error"); }); + afterEach(() => { errorStub.restore(); }); + + it("logs a readable message when there is no error object at all", () => { + testhubUtils.logBuildError(undefined, "observability"); + sinon.assert.calledWith(errorStub, "OBSERVABILITY Build creation failed"); + }); + + it("treats a null error the same as a missing one", () => { + testhubUtils.logBuildError(null, "observability"); + sinon.assert.calledWith(errorStub, "OBSERVABILITY Build creation failed"); + }); + }); +}); From a87bddc6624907da6e9706d72b197c81568ec989 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Fri, 11 Sep 2026 21:07:39 +0530 Subject: [PATCH 3/3] fix: gate support-file instrumentation on the string "true", not truthiness [SDK-7285] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BS_TESTOPS_BUILD_COMPLETED is a process.env value, so a failed build start stores the STRING "false" — which is truthy. Both setEventListeners gates therefore fired on the failure path and injected require('browserstack-cypress-cli/bin/testObservability/cypress') into the support file, while the matching npm_dependencies entry is only added on the success path. Remote webpack then cannot resolve it and EVERY spec fails to compile, so a TestHub build-start failure took the whole run down instead of just disabling observability. Measured on preprod: build=failed, 1 spec, 1 failed before; build=done, 1 spec, 2 passed after. Every other read of this variable already compares to the string explicitly (helper.js:557, 622, 680); these two gates were the outliers. Co-Authored-By: Claude Opus 5 --- bin/commands/runs.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/commands/runs.js b/bin/commands/runs.js index de99a0a2..daea68b7 100644 --- a/bin/commands/runs.js +++ b/bin/commands/runs.js @@ -212,7 +212,7 @@ module.exports = function run(args, rawArgs) { logger.debug("Completed setting the configs"); if(!isBrowserstackInfra) { - if(process.env.BS_TESTOPS_BUILD_COMPLETED) { + if(process.env.BS_TESTOPS_BUILD_COMPLETED === "true") { setEventListeners(bsConfig); } @@ -226,7 +226,7 @@ module.exports = function run(args, rawArgs) { if(process.env.BROWSERSTACK_TEST_ACCESSIBILITY === 'true') { setAccessibilityEventListeners(bsConfig); } - if(process.env.BS_TESTOPS_BUILD_COMPLETED) { + if(process.env.BS_TESTOPS_BUILD_COMPLETED === "true") { setEventListeners(bsConfig); } markBlockEnd('validateConfig');