From b5f9d49f21539a85cc54b65234cbfbca372a239f Mon Sep 17 00:00:00 2001 From: YASH JAIN Date: Thu, 30 Jul 2026 12:16:37 +0530 Subject: [PATCH 1/2] fix(observability): inject TRA support-file listener synchronously (SDK-7121) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit setEventListeners deferred the support-file write to an async glob callback while runs.js proceeded synchronously to md5 hashing and zip archiving. The injection raced the archive: a lost race shipped an un-instrumented suite, and md5 caching ("Skipping zip upload...") made the bad zip sticky, so the new Automate dashboard (TRA) received zero test events while the old dashboard (independent of the injected plugin) kept working. This is why the symptom was environment-specific — a slower CI pipeline loses the race. Switch setEventListeners (and the identical race in the accessibility setAccessibilityEventListeners glob branch) to glob.sync so the writes complete before the caller reads the files. Adds a regression test asserting the observability require is present synchronously after the call returns. Co-Authored-By: Claude Opus 4.8 --- bin/accessibility-automation/helper.js | 45 +++++++++--------- bin/testObservability/helper/helper.js | 40 ++++++++-------- .../testObservability/setEventListeners.js | 46 +++++++++++++++++++ 3 files changed, 88 insertions(+), 43 deletions(-) create mode 100644 test/unit/bin/testObservability/setEventListeners.js diff --git a/bin/accessibility-automation/helper.js b/bin/accessibility-automation/helper.js index 0cd0850f..9538f33e 100644 --- a/bin/accessibility-automation/helper.js +++ b/bin/accessibility-automation/helper.js @@ -378,32 +378,29 @@ exports.setAccessibilityEventListeners = (bsConfig) => { } const globPattern = process.cwd() + supportFilesData.supportFile; - glob(globPattern, {}, (err, files) => { - if(err) { - logger.debug('EXCEPTION IN BUILD START EVENT : Unable to parse cypress support files'); - return; - } - - files.forEach(file => { - try { - const fileName = path.basename(file); - if(['e2e.js', 'e2e.ts', 'component.ts', 'component.js'].includes(fileName) && !file.includes('node_modules')) { - - const defaultFileContent = fs.readFileSync(file, {encoding: 'utf-8'}); - let cypressCommandEventListener = getAccessibilityCypressCommandEventListener(path.extname(file)); - if(!defaultFileContent.includes(cypressCommandEventListener)) { - let newFileContent = defaultFileContent + - '\n' + - cypressCommandEventListener + - '\n'; - fs.writeFileSync(file, newFileContent, {encoding: 'utf-8'}); - supportFileContentMap[file] = supportFilesData.cleanupParams ? supportFilesData.cleanupParams : defaultFileContent; - } + // Synchronous for the same reason as testObservability setEventListeners + // (SDK-7121): the caller archives the suite right after this returns, so an + // async glob callback would race the archive and ship un-instrumented specs. + const files = glob.sync(globPattern, {}); + files.forEach(file => { + try { + const fileName = path.basename(file); + if(['e2e.js', 'e2e.ts', 'component.ts', 'component.js'].includes(fileName) && !file.includes('node_modules')) { + + const defaultFileContent = fs.readFileSync(file, {encoding: 'utf-8'}); + let cypressCommandEventListener = getAccessibilityCypressCommandEventListener(path.extname(file)); + if(!defaultFileContent.includes(cypressCommandEventListener)) { + let newFileContent = defaultFileContent + + '\n' + + cypressCommandEventListener + + '\n'; + fs.writeFileSync(file, newFileContent, {encoding: 'utf-8'}); + supportFileContentMap[file] = supportFilesData.cleanupParams ? supportFilesData.cleanupParams : defaultFileContent; } - } catch(e) { - logger.debug(`Unable to modify file contents for ${file} to set event listeners with error ${e}`, true, e); } - }); + } catch(e) { + logger.debug(`Unable to modify file contents for ${file} to set event listeners with error ${e}`, true, e); + } }); } catch(e) { logger.debug(`Unable to parse support files to set event listeners with error ${e}`, true, e); diff --git a/bin/testObservability/helper/helper.js b/bin/testObservability/helper/helper.js index 49d625a1..acc165a9 100644 --- a/bin/testObservability/helper/helper.js +++ b/bin/testObservability/helper/helper.js @@ -301,27 +301,29 @@ exports.setEventListeners = (bsConfig) => { try { const supportFilesData = helper.getSupportFiles(bsConfig, false); if(!supportFilesData.supportFile) return; - glob(process.cwd() + supportFilesData.supportFile, {}, (err, files) => { - if(err) return exports.debug('EXCEPTION IN BUILD START EVENT : Unable to parse cypress support files'); - files.forEach(file => { - try { - if (isE2ESupportFile(file) || !files.some(f => isE2ESupportFile(f))) { - const defaultFileContent = fs.readFileSync(file, {encoding: 'utf-8'}); - - let cypressCommandEventListener = getCypressCommandEventListener(file.includes('js')); - if(!defaultFileContent.includes(cypressCommandEventListener)) { - let newFileContent = defaultFileContent + - '\n' + - cypressCommandEventListener + - '\n' - fs.writeFileSync(file, newFileContent, {encoding: 'utf-8'}); - supportFileContentMap[file] = supportFilesData.cleanupParams ? supportFilesData.cleanupParams : defaultFileContent; - } + // Must be synchronous: runs.js proceeds to md5 hashing and zip archiving + // immediately after this returns. An async glob callback races the archive + // (SDK-7121) — a lost race ships an un-instrumented suite, and md5 caching + // makes it sticky, so TRA receives no test events. + const files = glob.sync(process.cwd() + supportFilesData.supportFile, {}); + files.forEach(file => { + try { + if (isE2ESupportFile(file) || !files.some(f => isE2ESupportFile(f))) { + const defaultFileContent = fs.readFileSync(file, {encoding: 'utf-8'}); + + let cypressCommandEventListener = getCypressCommandEventListener(file.includes('js')); + if(!defaultFileContent.includes(cypressCommandEventListener)) { + let newFileContent = defaultFileContent + + '\n' + + cypressCommandEventListener + + '\n' + fs.writeFileSync(file, newFileContent, {encoding: 'utf-8'}); + supportFileContentMap[file] = supportFilesData.cleanupParams ? supportFilesData.cleanupParams : defaultFileContent; } - } catch(e) { - exports.debug(`Unable to modify file contents for ${file} to set event listeners with error ${e}`, true, e); } - }); + } catch(e) { + exports.debug(`Unable to modify file contents for ${file} to set event listeners with error ${e}`, true, e); + } }); } catch(e) { exports.debug(`Unable to parse support files to set event listeners with error ${e}`, true, e); diff --git a/test/unit/bin/testObservability/setEventListeners.js b/test/unit/bin/testObservability/setEventListeners.js new file mode 100644 index 00000000..50f4431e --- /dev/null +++ b/test/unit/bin/testObservability/setEventListeners.js @@ -0,0 +1,46 @@ +'use strict'; +const chai = require('chai'); +const expect = chai.expect; +const sinon = require('sinon'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +const o11yHelper = require('../../../../bin/testObservability/helper/helper'); +const baseHelper = require('../../../../bin/helpers/helper'); + +// Regression guard for SDK-7121: the TRA support-file injection MUST land +// synchronously. runs.js calls setEventListeners(bsConfig) and then proceeds +// immediately to md5 hashing + zip archiving. When the injection was deferred to +// an async glob callback, it raced the archive — a lost race shipped an +// un-instrumented suite, and md5 caching made it sticky, so the new Automate +// dashboard (TRA) received zero test events. +describe('testObservability setEventListeners', () => { + let tmpDir, cwdStub, getSupportFilesStub; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sdk7121-')); + fs.mkdirSync(path.join(tmpDir, 'cypress', 'support'), { recursive: true }); + fs.writeFileSync(path.join(tmpDir, 'cypress', 'support', 'e2e.js'), '// user original support file\n'); + + cwdStub = sinon.stub(process, 'cwd').returns(tmpDir); + getSupportFilesStub = sinon.stub(baseHelper, 'getSupportFiles').returns({ + supportFile: '/cypress/support/e2e.js', + cleanupParams: {} + }); + }); + + afterEach(() => { + cwdStub.restore(); + getSupportFilesStub.restore(); + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('injects the observability require synchronously before returning', () => { + o11yHelper.setEventListeners({ run_settings: {} }); + + // Read exactly as md5/archive would — synchronously, right after the call. + const content = fs.readFileSync(path.join(tmpDir, 'cypress', 'support', 'e2e.js'), 'utf-8'); + expect(content).to.include("browserstack-cypress-cli/bin/testObservability/cypress"); + }); +}); From 47a6f3984f97b4c8fa5df3f4440c067c794b7943 Mon Sep 17 00:00:00 2001 From: YASH JAIN Date: Thu, 30 Jul 2026 13:43:30 +0530 Subject: [PATCH 2/2] test(observability): cover accessibility path + idempotency (SDK-7121) Address SDK PR Review coverage-gap finding on the regression test. The fix touched two files (setEventListeners and the accessibility setAccessibilityEventListeners glob branch) but the test only covered the observability path. Add: - accessibility setAccessibilityEventListeners synchronous-injection test (exercises the glob.sync pattern branch), - observability idempotency test (a second call does not double-inject). Co-Authored-By: Claude Opus 4.8 --- .../testObservability/setEventListeners.js | 72 ++++++++++++++----- 1 file changed, 53 insertions(+), 19 deletions(-) diff --git a/test/unit/bin/testObservability/setEventListeners.js b/test/unit/bin/testObservability/setEventListeners.js index 50f4431e..c3c32d62 100644 --- a/test/unit/bin/testObservability/setEventListeners.js +++ b/test/unit/bin/testObservability/setEventListeners.js @@ -7,40 +7,74 @@ const os = require('os'); const path = require('path'); const o11yHelper = require('../../../../bin/testObservability/helper/helper'); +const a11yHelper = require('../../../../bin/accessibility-automation/helper'); const baseHelper = require('../../../../bin/helpers/helper'); -// Regression guard for SDK-7121: the TRA support-file injection MUST land +// Regression guard for SDK-7121: the support-file instrumentation MUST land // synchronously. runs.js calls setEventListeners(bsConfig) and then proceeds // immediately to md5 hashing + zip archiving. When the injection was deferred to // an async glob callback, it raced the archive — a lost race shipped an // un-instrumented suite, and md5 caching made it sticky, so the new Automate // dashboard (TRA) received zero test events. -describe('testObservability setEventListeners', () => { - let tmpDir, cwdStub, getSupportFilesStub; +describe('SDK-7121 synchronous support-file instrumentation', () => { + let tmpDir, supportPath, cwdStub, getSupportFilesStub; - beforeEach(() => { + const setupTmpProject = () => { tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sdk7121-')); fs.mkdirSync(path.join(tmpDir, 'cypress', 'support'), { recursive: true }); - fs.writeFileSync(path.join(tmpDir, 'cypress', 'support', 'e2e.js'), '// user original support file\n'); - + supportPath = path.join(tmpDir, 'cypress', 'support', 'e2e.js'); + fs.writeFileSync(supportPath, '// user original support file\n'); cwdStub = sinon.stub(process, 'cwd').returns(tmpDir); - getSupportFilesStub = sinon.stub(baseHelper, 'getSupportFiles').returns({ - supportFile: '/cypress/support/e2e.js', - cleanupParams: {} - }); - }); + }; afterEach(() => { - cwdStub.restore(); - getSupportFilesStub.restore(); - fs.rmSync(tmpDir, { recursive: true, force: true }); + if (cwdStub) cwdStub.restore(); + if (getSupportFilesStub) getSupportFilesStub.restore(); + if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true }); + cwdStub = getSupportFilesStub = tmpDir = undefined; + }); + + describe('testObservability setEventListeners', () => { + beforeEach(() => { + setupTmpProject(); + process.env.BS_TESTOPS_BUILD_COMPLETED = 'true'; + // non-magic path -> glob.sync resolves the exact file + getSupportFilesStub = sinon.stub(baseHelper, 'getSupportFiles').returns({ + supportFile: '/cypress/support/e2e.js', + cleanupParams: {} + }); + }); + + it('injects the observability require synchronously before returning', () => { + o11yHelper.setEventListeners({ run_settings: {} }); + // Read exactly as md5/archive would — synchronously, right after the call. + const content = fs.readFileSync(supportPath, 'utf-8'); + expect(content).to.include('browserstack-cypress-cli/bin/testObservability/cypress'); + }); + + it('does not double-inject when called twice (idempotent)', () => { + o11yHelper.setEventListeners({ run_settings: {} }); + o11yHelper.setEventListeners({ run_settings: {} }); + const content = fs.readFileSync(supportPath, 'utf-8'); + const occurrences = content.split('browserstack-cypress-cli/bin/testObservability/cypress').length - 1; + expect(occurrences).to.equal(1); + }); }); - it('injects the observability require synchronously before returning', () => { - o11yHelper.setEventListeners({ run_settings: {} }); + describe('accessibility setAccessibilityEventListeners (glob-pattern branch)', () => { + beforeEach(() => { + setupTmpProject(); + // magic pattern -> exercises the glob.sync branch fixed for SDK-7121 + getSupportFilesStub = sinon.stub(baseHelper, 'getSupportFiles').returns({ + supportFile: '/cypress/support/**/*.js', + cleanupParams: {} + }); + }); - // Read exactly as md5/archive would — synchronously, right after the call. - const content = fs.readFileSync(path.join(tmpDir, 'cypress', 'support', 'e2e.js'), 'utf-8'); - expect(content).to.include("browserstack-cypress-cli/bin/testObservability/cypress"); + it('injects the accessibility require synchronously before returning', () => { + a11yHelper.setAccessibilityEventListeners({ run_settings: {} }); + const content = fs.readFileSync(supportPath, 'utf-8'); + expect(content).to.include('browserstack-cypress-cli/bin/accessibility-automation/cypress'); + }); }); });