From 613a13687bd1b5e9da388f979db890c7762e23bc Mon Sep 17 00:00:00 2001 From: Bhargavi-BS Date: Mon, 6 Jul 2026 02:19:47 +0530 Subject: [PATCH 1/3] perf(upload): prune excluded directories from zip/md5 walks + cap telemetry size walk (SDK-6463) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On large monorepos the CLI stalled for minutes before uploading: 1. archiver.glob and the spec-md5 walk (hashUtil via checkUploaded) pass excludes as readdir-glob 'ignore', which filters entries only AFTER the walker has descended into and lstat'ed every file under node_modules/.git/dist/etc. The customer's log showed ~86s in 'Creating tests.zip' and a similar hidden cost in the md5 step. 2. utils.fetchFolderSize(node_modules) — telemetry only — recursively stats the entire node_modules between archiving and uploading. Fix: reuse every ignore pattern ending in '/**' as readdir-glob's 'skip' option (getDirectorySkipPatterns), which prevents descending into matching directories. This is provably safe: a directory matching '/**' means every descendant also matches, so pruning cannot change the archive contents or the md5. Verified on a synthetic 144k-file monorepo with the customer's exact 130 exclude patterns: zip 7.5s -> 0.1s and md5 5.5s -> 0.0s, with byte-identical zip entry lists and an UNCHANGED md5 (upload cache keys unaffected). Also adds a 5s cooperative deadline to the folder-size telemetry walk so it can never stall a run. Co-Authored-By: Claude Fable 5 --- bin/helpers/archiver.js | 7 ++++++- bin/helpers/checkUploaded.js | 3 +++ bin/helpers/utils.js | 34 ++++++++++++++++++++++++++++++---- test/unit/bin/helpers/utils.js | 28 ++++++++++++++++++++++++++++ 4 files changed, 67 insertions(+), 5 deletions(-) diff --git a/bin/helpers/archiver.js b/bin/helpers/archiver.js index fb3b3155..1b6cd942 100644 --- a/bin/helpers/archiver.js +++ b/bin/helpers/archiver.js @@ -58,7 +58,12 @@ const archiveSpecs = (runSettings, filePath, excludeFiles, md5data) => { let ignoreFiles = utils.getFilesToIgnore(runSettings, excludeFiles); logger.debug(`Patterns ignored during zip ${ignoreFiles}`); - archive.glob(`**/*.+(${Constants.allowedFileTypes.join("|")})`, { cwd: cypressFolderPath, matchBase: true, ignore: ignoreFiles, dot:true }); + // SDK-6463 (perf): `ignore` filters entries only AFTER the walk visits them, so the + // globber still descends into node_modules/.git/etc. `skip` prunes those directories + // from the traversal entirely — on large monorepos this cuts zip creation from + // minutes to seconds without changing the archive contents. + let skipDirectories = utils.getDirectorySkipPatterns(ignoreFiles); + archive.glob(`**/*.+(${Constants.allowedFileTypes.join("|")})`, { cwd: cypressFolderPath, matchBase: true, ignore: ignoreFiles, skip: skipDirectories, dot:true }); let packageJSON = {}; diff --git a/bin/helpers/checkUploaded.js b/bin/helpers/checkUploaded.js index dfc29241..39895dd4 100644 --- a/bin/helpers/checkUploaded.js +++ b/bin/helpers/checkUploaded.js @@ -25,6 +25,9 @@ const checkSpecsMd5 = (runSettings, args, instrumentBlocks) => { let options = { cwd: cypressFolderPath, ignore: ignoreFiles, + // SDK-6463 (perf): prune ignored directories from the md5 walk instead of + // filtering entries after descending into them (see utils.getDirectorySkipPatterns). + skip: utils.getDirectorySkipPatterns(ignoreFiles), pattern: `**/*.+(${Constants.allowedFileTypes.join("|")})` }; hashHelper.hashWrapper(options, instrumentBlocks).then(function (data) { diff --git a/bin/helpers/utils.js b/bin/helpers/utils.js index 3ec867b9..22344a2e 100644 --- a/bin/helpers/utils.js +++ b/bin/helpers/utils.js @@ -1103,6 +1103,20 @@ exports.getFilesToIgnore = (runSettings, excludeFiles, logging = true) => { return ignoreFiles; } +// SDK-6463 (perf): derive directory-pruning patterns from the ignore list. +// readdir-glob (used both by archiver.glob for tests.zip and by hashUtil for the +// spec md5 check) applies `ignore` per-entry AFTER walking, so it still descends +// into node_modules/.git/dist etc. On large monorepos that walk alone takes +// minutes ("Creating tests.zip" stalls). Its `skip` option instead prevents +// DESCENDING into matching directories. Any ignore pattern ending in '/**' is +// safe to reuse as a skip: if a directory matches '/**' then every descendant +// also matches it, so pruning cannot change which files end up included. +exports.getDirectorySkipPatterns = (ignoreFiles) => { + return (ignoreFiles || []).filter((pattern) => + typeof pattern === 'string' && pattern.endsWith('/**') + ); +} + exports.getNumberOfSpecFiles = (bsConfig, args, cypressConfig, turboScaleSession=false) => { let defaultSpecFolder let testFolderPath @@ -1722,13 +1736,20 @@ exports.fetchZipSize = (fileName) => { } } -const getDirectorySize = async function(dir) { +const getDirectorySize = async function(dir, deadline) { try{ + // SDK-6463 (perf): this telemetry-only walk recursively stats every file (it is + // pointed at node_modules in runs.js). On large monorepos it blocked the pipeline + // between archiving and uploading for tens of seconds. Stop descending once the + // deadline passes — folder size is best-effort telemetry, never worth stalling a run. + if (deadline && Date.now() > deadline) { + return 0; + } const subdirs = (await readdir(dir)); const files = await Promise.all(subdirs.map(async (subdir) => { const res = path.resolve(dir, subdir); const s = (await stat(res)); - return s.isDirectory() ? getDirectorySize(res) : (s.size); + return s.isDirectory() ? getDirectorySize(res, deadline) : (s.size); })); return files.reduce((a, f) => a+f, 0); }catch(e){ @@ -1738,10 +1759,15 @@ const getDirectorySize = async function(dir) { } }; -exports.fetchFolderSize = async (dir) => { +exports.fetchFolderSize = async (dir, timeoutMs = 5000) => { try { if(fs.existsSync(dir)){ - return (await getDirectorySize(dir) / 1024 / 1024); + const deadline = Date.now() + timeoutMs; + const size = (await getDirectorySize(dir, deadline) / 1024 / 1024); + if (Date.now() > deadline) { + logger.debug(`Folder size calculation for ${dir} exceeded ${timeoutMs}ms; reporting partial size.`); + } + return size; } return 0; } catch (error) { diff --git a/test/unit/bin/helpers/utils.js b/test/unit/bin/helpers/utils.js index 1d24ced6..9a1d4cdf 100644 --- a/test/unit/bin/helpers/utils.js +++ b/test/unit/bin/helpers/utils.js @@ -770,6 +770,34 @@ describe('utils', () => { }); }); + // SDK-6463 (perf): directory-shaped ignore patterns are reused as readdir-glob `skip` + // patterns so the zip/md5 walks prune excluded trees instead of descending into them. + describe('getDirectorySkipPatterns', () => { + it('keeps only patterns ending in /** (safe to prune)', () => { + const ignore = [ + '**/node_modules/**', + 'node_modules/**', + 'dist/**', + 'apps/admin-portal/**', + 'package.json', // file pattern — must NOT be used for pruning + '.env', + '**/README.md', + ]; + chai.expect(utils.getDirectorySkipPatterns(ignore)).to.be.eql([ + '**/node_modules/**', + 'node_modules/**', + 'dist/**', + 'apps/admin-portal/**', + ]); + }); + + it('handles empty/undefined input and non-string entries', () => { + chai.expect(utils.getDirectorySkipPatterns(undefined)).to.be.eql([]); + chai.expect(utils.getDirectorySkipPatterns([])).to.be.eql([]); + chai.expect(utils.getDirectorySkipPatterns([null, 42, 'x/**'])).to.be.eql(['x/**']); + }); + }); + describe('setTestEnvs', () => { it('set env only from args', () => { let argsEnv = 'env3=value3, env4=value4'; From dcac4df39add220aa8b059c59fc8796e7364711e Mon Sep 17 00:00:00 2001 From: Bhargavi-BS Date: Mon, 6 Jul 2026 13:47:59 +0530 Subject: [PATCH 2/3] chore: remove ticket identifiers from code comments Same review convention as requested on #1131. Co-Authored-By: Claude Fable 5 --- bin/helpers/archiver.js | 2 +- bin/helpers/checkUploaded.js | 2 +- bin/helpers/utils.js | 4 ++-- test/unit/bin/helpers/utils.js | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bin/helpers/archiver.js b/bin/helpers/archiver.js index 1b6cd942..a30c072e 100644 --- a/bin/helpers/archiver.js +++ b/bin/helpers/archiver.js @@ -58,7 +58,7 @@ const archiveSpecs = (runSettings, filePath, excludeFiles, md5data) => { let ignoreFiles = utils.getFilesToIgnore(runSettings, excludeFiles); logger.debug(`Patterns ignored during zip ${ignoreFiles}`); - // SDK-6463 (perf): `ignore` filters entries only AFTER the walk visits them, so the + // Perf: `ignore` filters entries only AFTER the walk visits them, so the // globber still descends into node_modules/.git/etc. `skip` prunes those directories // from the traversal entirely — on large monorepos this cuts zip creation from // minutes to seconds without changing the archive contents. diff --git a/bin/helpers/checkUploaded.js b/bin/helpers/checkUploaded.js index 39895dd4..9578d127 100644 --- a/bin/helpers/checkUploaded.js +++ b/bin/helpers/checkUploaded.js @@ -25,7 +25,7 @@ const checkSpecsMd5 = (runSettings, args, instrumentBlocks) => { let options = { cwd: cypressFolderPath, ignore: ignoreFiles, - // SDK-6463 (perf): prune ignored directories from the md5 walk instead of + // Perf: prune ignored directories from the md5 walk instead of // filtering entries after descending into them (see utils.getDirectorySkipPatterns). skip: utils.getDirectorySkipPatterns(ignoreFiles), pattern: `**/*.+(${Constants.allowedFileTypes.join("|")})` diff --git a/bin/helpers/utils.js b/bin/helpers/utils.js index 22344a2e..2d5fe8b0 100644 --- a/bin/helpers/utils.js +++ b/bin/helpers/utils.js @@ -1103,7 +1103,7 @@ exports.getFilesToIgnore = (runSettings, excludeFiles, logging = true) => { return ignoreFiles; } -// SDK-6463 (perf): derive directory-pruning patterns from the ignore list. +// Perf: derive directory-pruning patterns from the ignore list. // readdir-glob (used both by archiver.glob for tests.zip and by hashUtil for the // spec md5 check) applies `ignore` per-entry AFTER walking, so it still descends // into node_modules/.git/dist etc. On large monorepos that walk alone takes @@ -1738,7 +1738,7 @@ exports.fetchZipSize = (fileName) => { const getDirectorySize = async function(dir, deadline) { try{ - // SDK-6463 (perf): this telemetry-only walk recursively stats every file (it is + // Perf: this telemetry-only walk recursively stats every file (it is // pointed at node_modules in runs.js). On large monorepos it blocked the pipeline // between archiving and uploading for tens of seconds. Stop descending once the // deadline passes — folder size is best-effort telemetry, never worth stalling a run. diff --git a/test/unit/bin/helpers/utils.js b/test/unit/bin/helpers/utils.js index 9a1d4cdf..9360fb90 100644 --- a/test/unit/bin/helpers/utils.js +++ b/test/unit/bin/helpers/utils.js @@ -770,7 +770,7 @@ describe('utils', () => { }); }); - // SDK-6463 (perf): directory-shaped ignore patterns are reused as readdir-glob `skip` + // Perf: directory-shaped ignore patterns are reused as readdir-glob `skip` // patterns so the zip/md5 walks prune excluded trees instead of descending into them. describe('getDirectorySkipPatterns', () => { it('keeps only patterns ending in /** (safe to prune)', () => { From 609d50d2ff680f267525420612b64776e6b8a936 Mon Sep 17 00:00:00 2001 From: Bhargavi-BS Date: Mon, 6 Jul 2026 14:14:04 +0530 Subject: [PATCH 3/3] perf(upload): compute node_modules telemetry size concurrently with the upload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The node_modules size is instrumentation-only, so there is no reason to block the upload on walking the tree. Start the walk before the upload and await it only after the upload completes — in the common case it resolves while the upload is in flight, adding zero wall-clock. The 5s deadline in fetchFolderSize remains as a backstop for trees whose walk outlives the upload. fetchFolderSize never rejects, so the floating promise is safe. Co-Authored-By: Claude Fable 5 --- bin/commands/runs.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/bin/commands/runs.js b/bin/commands/runs.js index df02954a..2f58a904 100644 --- a/bin/commands/runs.js +++ b/bin/commands/runs.js @@ -279,7 +279,11 @@ module.exports = function run(args, rawArgs) { let test_zip_size = utils.fetchZipSize(path.join(process.cwd(), config.fileName)); let npm_zip_size = utils.fetchZipSize(path.join(process.cwd(), config.packageFileName)); - let node_modules_size = await utils.fetchFolderSize(path.join(process.cwd(), "node_modules")); + // Perf: node_modules size is instrumentation-only, so don't block the upload on + // walking the tree — start the walk here and await it only after the upload has + // completed (in the common case it resolves while the upload is in flight, adding + // zero wall-clock; fetchFolderSize never rejects, so the floating promise is safe). + let nodeModulesSizePromise = utils.fetchFolderSize(path.join(process.cwd(), "node_modules")); if (Constants.turboScaleObj.enabled) { // Note: Calculating md5 here for turboscale force-upload so that we don't need to re-calculate at hub @@ -306,6 +310,9 @@ module.exports = function run(args, rawArgs) { markBlockEnd('zip.zipUpload'); markBlockEnd('zip'); + // Walk was started before the upload; usually already resolved by now. + let node_modules_size = await nodeModulesSizePromise; + if (process.env.BROWSERSTACK_TEST_ACCESSIBILITY === 'true') { supportFileCleanup(); }