From 312a5d9f30ff95bfacd70ff69b27128173d14aa0 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 28 Jan 2023 15:47:10 +0100 Subject: [PATCH 1/2] cli: make sure max workers workers are set to at least 2 by default Signed-off-by: Patrik Oldsberg --- .changeset/chatty-owls-care.md | 2 +- packages/cli/src/commands/repo/test.ts | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/.changeset/chatty-owls-care.md b/.changeset/chatty-owls-care.md index 84b6fc76f6..9f39f51bb5 100644 --- a/.changeset/chatty-owls-care.md +++ b/.changeset/chatty-owls-care.md @@ -2,4 +2,4 @@ '@backstage/cli': patch --- -The `backstage-cli repo test` command now sets a default Jest `--workerIdleMemoryLimit` of 1GB. This is the recommended workaround for dealing with Jest workers leaking memory and eventually hitting the heap limit. +The `backstage-cli repo test` command now sets a default Jest `--workerIdleMemoryLimit` of 1GB. If needed to ensure that tests are not run in band, `--maxWorkers=2` is set as well. This is the recommended workaround for dealing with Jest workers leaking memory and eventually hitting the heap limit. diff --git a/packages/cli/src/commands/repo/test.ts b/packages/cli/src/commands/repo/test.ts index 69b3d914ed..22f1a68745 100644 --- a/packages/cli/src/commands/repo/test.ts +++ b/packages/cli/src/commands/repo/test.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import os from 'os'; import { Command, OptionValues } from 'commander'; import { PackageGraph } from '../../lib/monorepo'; import { paths } from '../../lib/paths'; @@ -91,6 +92,19 @@ export async function command(opts: OptionValues, cmd: Command): Promise { args.push('--workerIdleMemoryLimit=1000M'); } + // In order for the above worker memory limit to work we need to make sure the worker + // count is set to at least 2, as the tests will otherwise run in-band. + // Depending on the mode tests are run with the default count is either cpus-1, or cpus/2. + // This means that if we've got at 4 or more cores we'll always get at least 2 workers, but + // otherwise we need to set the worker count explicitly unless already done. + if ( + os.cpus().length <= 3 && + !includesAnyOf(args, '-i', '--runInBand') && + !args.some(arg => arg.match(/^(--maxWorkers|-w)/)) + ) { + args.push('--maxWorkers=2'); + } + if (opts.since) { removeOptionArg(args, '--since'); } From 32e280c81ac0cfca5b731c80fe5a644d185bd442 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 28 Jan 2023 17:33:57 +0100 Subject: [PATCH 2/2] cli: refactor flag detection in repo test Signed-off-by: Patrik Oldsberg --- packages/cli/src/commands/repo/test.test.ts | 47 ++++++++++++++++++++ packages/cli/src/commands/repo/test.ts | 49 +++++++++++++-------- 2 files changed, 77 insertions(+), 19 deletions(-) create mode 100644 packages/cli/src/commands/repo/test.test.ts diff --git a/packages/cli/src/commands/repo/test.test.ts b/packages/cli/src/commands/repo/test.test.ts new file mode 100644 index 0000000000..d2f2485717 --- /dev/null +++ b/packages/cli/src/commands/repo/test.test.ts @@ -0,0 +1,47 @@ +/* + * Copyright 2020 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { createFlagFinder } from './test'; + +describe('createFlagFinder', () => { + it('finds flags', () => { + const find = createFlagFinder([ + '--foo', + '--no-bar', + '-b', + '-c', + '--baz=1', + '--qux', + '2', + '-de', + ]); + + expect( + find('--foo', '--bar', '-b', '-c', '--baz', '--qux', '-d', '-e'), + ).toBe(true); + expect(find('--foo')).toBe(true); + expect(find('--bar')).toBe(true); + expect(find('--no-bar')).toBe(false); + expect(find('-a')).toBe(false); + expect(find('-b')).toBe(true); + expect(find('-c')).toBe(true); + expect(find('-d')).toBe(true); + expect(find('-e')).toBe(true); + expect(find('--baz')).toBe(true); + expect(find('--qux')).toBe(true); + expect(find('--qux')).toBe(true); + }); +}); diff --git a/packages/cli/src/commands/repo/test.ts b/packages/cli/src/commands/repo/test.ts index 22f1a68745..2dd778677a 100644 --- a/packages/cli/src/commands/repo/test.ts +++ b/packages/cli/src/commands/repo/test.ts @@ -20,13 +20,30 @@ import { PackageGraph } from '../../lib/monorepo'; import { paths } from '../../lib/paths'; import { runCheck } from '../../lib/run'; -function includesAnyOf(hayStack: string[], ...needles: string[]) { - for (const needle of needles) { - if (hayStack.includes(needle)) { - return true; +export function createFlagFinder(args: string[]) { + const flags = new Set(); + + for (const arg of args) { + if (arg.startsWith('--no-')) { + flags.add(`--${arg.slice('--no-'.length)}`); + } else if (arg.startsWith('--')) { + flags.add(arg.split('=')[0]); + } else if (arg.startsWith('-')) { + const shortFlags = arg.slice(1).split(''); + for (const shortFlag of shortFlags) { + flags.add(`-${shortFlag}`); + } } } - return false; + + return (...findFlags: string[]) => { + for (const flag of findFlags) { + if (flags.has(flag)) { + return true; + } + } + return false; + }; } function removeOptionArg(args: string[], option: string) { @@ -56,24 +73,19 @@ export async function command(opts: OptionValues, cmd: Command): Promise { const allArgs = parent.args as string[]; const args = allArgs.slice(allArgs.indexOf('test') + 1); + const hasFlags = createFlagFinder(args); + // Only include our config if caller isn't passing their own config - if (!includesAnyOf(args, '-c', '--config')) { + if (!hasFlags('-c', '--config')) { args.push('--config', paths.resolveOwn('config/jest.js')); } - if (!includesAnyOf(args, '--no-passWithNoTests', '--passWithNoTests=false')) { + if (!hasFlags('--passWithNoTests')) { args.push('--passWithNoTests'); } // Run in watch mode unless in CI, coverage mode, or running all tests - if ( - !process.env.CI && - !args.includes('--coverage') && - // explicitly no watching - !includesAnyOf(args, '--no-watch', '--watch=false', '--watchAll=false') && - // already watching - !includesAnyOf(args, '--watch', '--watchAll') - ) { + if (!process.env.CI && !hasFlags('--coverage', '--watch', '--watchAll')) { const isGitRepo = () => runCheck('git', 'rev-parse', '--is-inside-work-tree'); const isMercurialRepo = () => runCheck('hg', '--cwd', '.', 'root'); @@ -88,7 +100,7 @@ export async function command(opts: OptionValues, cmd: Command): Promise { // When running tests from the repo root in large repos you can easily hit the heap limit. // This is because Jest workers leak a lot of memory, and the workaround is to limit worker memory. // We set a default memory limit, but if an explicit one is supplied it will be used instead - if (!args.some(arg => arg.match(/^--workerIdleMemoryLimit/))) { + if (!hasFlags('--workerIdleMemoryLimit')) { args.push('--workerIdleMemoryLimit=1000M'); } @@ -99,8 +111,7 @@ export async function command(opts: OptionValues, cmd: Command): Promise { // otherwise we need to set the worker count explicitly unless already done. if ( os.cpus().length <= 3 && - !includesAnyOf(args, '-i', '--runInBand') && - !args.some(arg => arg.match(/^(--maxWorkers|-w)/)) + !hasFlags('-i', '--runInBand', '-w', '--maxWorkers') ) { args.push('--maxWorkers=2'); } @@ -109,7 +120,7 @@ export async function command(opts: OptionValues, cmd: Command): Promise { removeOptionArg(args, '--since'); } - if (opts.since && !args.some(arg => arg.startsWith('--selectProjects'))) { + if (opts.since && !hasFlags('--selectProjects')) { const packages = await PackageGraph.listTargetPackages(); const graph = PackageGraph.fromPackages(packages); const changedPackages = await graph.listChangedPackages({