Skip to content

Commit f446349

Browse files
authored
Merge pull request #1160 from browserstack/fix/sdk-7121-tra-support-file-race
fix(observability): inject TRA support-file listener synchronously (SDK-7121)
2 parents 9056143 + 3de3f02 commit f446349

3 files changed

Lines changed: 128 additions & 43 deletions

File tree

bin/accessibility-automation/helper.js

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -378,32 +378,35 @@ exports.setAccessibilityEventListeners = (bsConfig) => {
378378
}
379379

380380
const globPattern = process.cwd() + supportFilesData.supportFile;
381-
glob(globPattern, {}, (err, files) => {
382-
if(err) {
383-
logger.debug('EXCEPTION IN BUILD START EVENT : Unable to parse cypress support files');
384-
return;
385-
}
386-
387-
files.forEach(file => {
388-
try {
389-
const fileName = path.basename(file);
390-
if(['e2e.js', 'e2e.ts', 'component.ts', 'component.js'].includes(fileName) && !file.includes('node_modules')) {
391-
392-
const defaultFileContent = fs.readFileSync(file, {encoding: 'utf-8'});
393-
let cypressCommandEventListener = getAccessibilityCypressCommandEventListener(path.extname(file));
394-
if(!defaultFileContent.includes(cypressCommandEventListener)) {
395-
let newFileContent = defaultFileContent +
396-
'\n' +
397-
cypressCommandEventListener +
398-
'\n';
399-
fs.writeFileSync(file, newFileContent, {encoding: 'utf-8'});
400-
supportFileContentMap[file] = supportFilesData.cleanupParams ? supportFilesData.cleanupParams : defaultFileContent;
401-
}
381+
// Synchronous for the same reason as testObservability setEventListeners
382+
// (SDK-7121): the caller archives the suite right after this returns, so an
383+
// async glob callback would race the archive and ship un-instrumented specs.
384+
let files;
385+
try {
386+
files = glob.sync(globPattern, {});
387+
} catch(err) {
388+
logger.debug('EXCEPTION IN BUILD START EVENT : Unable to parse cypress support files', true, err);
389+
return;
390+
}
391+
files.forEach(file => {
392+
try {
393+
const fileName = path.basename(file);
394+
if(['e2e.js', 'e2e.ts', 'component.ts', 'component.js'].includes(fileName) && !file.includes('node_modules')) {
395+
396+
const defaultFileContent = fs.readFileSync(file, {encoding: 'utf-8'});
397+
let cypressCommandEventListener = getAccessibilityCypressCommandEventListener(path.extname(file));
398+
if(!defaultFileContent.includes(cypressCommandEventListener)) {
399+
let newFileContent = defaultFileContent +
400+
'\n' +
401+
cypressCommandEventListener +
402+
'\n';
403+
fs.writeFileSync(file, newFileContent, {encoding: 'utf-8'});
404+
supportFileContentMap[file] = supportFilesData.cleanupParams ? supportFilesData.cleanupParams : defaultFileContent;
402405
}
403-
} catch(e) {
404-
logger.debug(`Unable to modify file contents for ${file} to set event listeners with error ${e}`, true, e);
405406
}
406-
});
407+
} catch(e) {
408+
logger.debug(`Unable to modify file contents for ${file} to set event listeners with error ${e}`, true, e);
409+
}
407410
});
408411
} catch(e) {
409412
logger.debug(`Unable to parse support files to set event listeners with error ${e}`, true, e);

bin/testObservability/helper/helper.js

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -301,27 +301,29 @@ exports.setEventListeners = (bsConfig) => {
301301
try {
302302
const supportFilesData = helper.getSupportFiles(bsConfig, false);
303303
if(!supportFilesData.supportFile) return;
304-
glob(process.cwd() + supportFilesData.supportFile, {}, (err, files) => {
305-
if(err) return exports.debug('EXCEPTION IN BUILD START EVENT : Unable to parse cypress support files');
306-
files.forEach(file => {
307-
try {
308-
if (isE2ESupportFile(file) || !files.some(f => isE2ESupportFile(f))) {
309-
const defaultFileContent = fs.readFileSync(file, {encoding: 'utf-8'});
310-
311-
let cypressCommandEventListener = getCypressCommandEventListener(file.includes('js'));
312-
if(!defaultFileContent.includes(cypressCommandEventListener)) {
313-
let newFileContent = defaultFileContent +
314-
'\n' +
315-
cypressCommandEventListener +
316-
'\n'
317-
fs.writeFileSync(file, newFileContent, {encoding: 'utf-8'});
318-
supportFileContentMap[file] = supportFilesData.cleanupParams ? supportFilesData.cleanupParams : defaultFileContent;
319-
}
304+
// Must be synchronous: runs.js proceeds to md5 hashing and zip archiving
305+
// immediately after this returns. An async glob callback races the archive
306+
// (SDK-7121) — a lost race ships an un-instrumented suite, and md5 caching
307+
// makes it sticky, so TRA receives no test events.
308+
const files = glob.sync(process.cwd() + supportFilesData.supportFile, {});
309+
files.forEach(file => {
310+
try {
311+
if (isE2ESupportFile(file) || !files.some(f => isE2ESupportFile(f))) {
312+
const defaultFileContent = fs.readFileSync(file, {encoding: 'utf-8'});
313+
314+
let cypressCommandEventListener = getCypressCommandEventListener(file.includes('js'));
315+
if(!defaultFileContent.includes(cypressCommandEventListener)) {
316+
let newFileContent = defaultFileContent +
317+
'\n' +
318+
cypressCommandEventListener +
319+
'\n'
320+
fs.writeFileSync(file, newFileContent, {encoding: 'utf-8'});
321+
supportFileContentMap[file] = supportFilesData.cleanupParams ? supportFilesData.cleanupParams : defaultFileContent;
320322
}
321-
} catch(e) {
322-
exports.debug(`Unable to modify file contents for ${file} to set event listeners with error ${e}`, true, e);
323323
}
324-
});
324+
} catch(e) {
325+
exports.debug(`Unable to modify file contents for ${file} to set event listeners with error ${e}`, true, e);
326+
}
325327
});
326328
} catch(e) {
327329
exports.debug(`Unable to parse support files to set event listeners with error ${e}`, true, e);
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
'use strict';
2+
const chai = require('chai');
3+
const expect = chai.expect;
4+
const sinon = require('sinon');
5+
const fs = require('fs');
6+
const os = require('os');
7+
const path = require('path');
8+
9+
const o11yHelper = require('../../../../bin/testObservability/helper/helper');
10+
const a11yHelper = require('../../../../bin/accessibility-automation/helper');
11+
const baseHelper = require('../../../../bin/helpers/helper');
12+
13+
// Regression guard for SDK-7121: the support-file instrumentation MUST land
14+
// synchronously. runs.js calls setEventListeners(bsConfig) and then proceeds
15+
// immediately to md5 hashing + zip archiving. When the injection was deferred to
16+
// an async glob callback, it raced the archive — a lost race shipped an
17+
// un-instrumented suite, and md5 caching made it sticky, so the new Automate
18+
// dashboard (TRA) received zero test events.
19+
describe('SDK-7121 synchronous support-file instrumentation', () => {
20+
let tmpDir, supportPath, cwdStub, getSupportFilesStub;
21+
22+
const setupTmpProject = () => {
23+
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sdk7121-'));
24+
fs.mkdirSync(path.join(tmpDir, 'cypress', 'support'), { recursive: true });
25+
supportPath = path.join(tmpDir, 'cypress', 'support', 'e2e.js');
26+
fs.writeFileSync(supportPath, '// user original support file\n');
27+
cwdStub = sinon.stub(process, 'cwd').returns(tmpDir);
28+
};
29+
30+
afterEach(() => {
31+
if (cwdStub) cwdStub.restore();
32+
if (getSupportFilesStub) getSupportFilesStub.restore();
33+
if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true });
34+
cwdStub = getSupportFilesStub = tmpDir = undefined;
35+
});
36+
37+
describe('testObservability setEventListeners', () => {
38+
beforeEach(() => {
39+
setupTmpProject();
40+
process.env.BS_TESTOPS_BUILD_COMPLETED = 'true';
41+
// non-magic path -> glob.sync resolves the exact file
42+
getSupportFilesStub = sinon.stub(baseHelper, 'getSupportFiles').returns({
43+
supportFile: '/cypress/support/e2e.js',
44+
cleanupParams: {}
45+
});
46+
});
47+
48+
it('injects the observability require synchronously before returning', () => {
49+
o11yHelper.setEventListeners({ run_settings: {} });
50+
// Read exactly as md5/archive would — synchronously, right after the call.
51+
const content = fs.readFileSync(supportPath, 'utf-8');
52+
expect(content).to.include('browserstack-cypress-cli/bin/testObservability/cypress');
53+
});
54+
55+
it('does not double-inject when called twice (idempotent)', () => {
56+
o11yHelper.setEventListeners({ run_settings: {} });
57+
o11yHelper.setEventListeners({ run_settings: {} });
58+
const content = fs.readFileSync(supportPath, 'utf-8');
59+
const occurrences = content.split('browserstack-cypress-cli/bin/testObservability/cypress').length - 1;
60+
expect(occurrences).to.equal(1);
61+
});
62+
});
63+
64+
describe('accessibility setAccessibilityEventListeners (glob-pattern branch)', () => {
65+
beforeEach(() => {
66+
setupTmpProject();
67+
// magic pattern -> exercises the glob.sync branch fixed for SDK-7121
68+
getSupportFilesStub = sinon.stub(baseHelper, 'getSupportFiles').returns({
69+
supportFile: '/cypress/support/**/*.js',
70+
cleanupParams: {}
71+
});
72+
});
73+
74+
it('injects the accessibility require synchronously before returning', () => {
75+
a11yHelper.setAccessibilityEventListeners({ run_settings: {} });
76+
const content = fs.readFileSync(supportPath, 'utf-8');
77+
expect(content).to.include('browserstack-cypress-cli/bin/accessibility-automation/cypress');
78+
});
79+
});
80+
});

0 commit comments

Comments
 (0)