From a0eebb82c9ef8385cecd4a5f6c0d1ebd22561789 Mon Sep 17 00:00:00 2001 From: David Olaru Date: Thu, 2 Jan 2025 17:10:20 +0000 Subject: [PATCH] [ftr] Speed up FTR code owner check (#205093) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Switch to one matcher w/ all code owner patterns rather than separate matchers for each code owner pattern. Reduces the run time of `scripts/check_ftr_code_owners.js` by ~10x. ### Before ```console ▶ node scripts/check_ftr_code_owners.js info Reading CODEOWNERS file info Checking ownership for 8653 test files (this will take a while) info Ownership check complete (took 18.89 s) succ All test files have a code owner. 🥳 ``` #### After ```console ▶ node scripts/check_ftr_code_owners.js info Checked 8653 test files in 1.59s succ All test files have a code owner 🥳 ``` --- packages/kbn-test/index.ts | 2 - .../functional_test_runner/cli/code_owners.ts | 48 +++++++++++++ .../{cli.ts => cli/ftr.ts} | 4 +- .../src/functional_test_runner/cli/index.ts | 11 +++ .../src/functional_test_runner/index.ts | 2 +- .../run_check_ftr_code_owners.ts | 67 ------------------- scripts/check_ftr_code_owners.js | 2 +- 7 files changed, 63 insertions(+), 73 deletions(-) create mode 100644 packages/kbn-test/src/functional_test_runner/cli/code_owners.ts rename packages/kbn-test/src/functional_test_runner/{cli.ts => cli/ftr.ts} (98%) create mode 100644 packages/kbn-test/src/functional_test_runner/cli/index.ts delete mode 100644 packages/kbn-test/src/functional_test_runner/run_check_ftr_code_owners.ts diff --git a/packages/kbn-test/index.ts b/packages/kbn-test/index.ts index b5506cc804ad3..cc97a4afa7906 100644 --- a/packages/kbn-test/index.ts +++ b/packages/kbn-test/index.ts @@ -69,8 +69,6 @@ export { getUrl } from './src/jest/get_url'; export { runCheckJestConfigsCli } from './src/jest/run_check_jest_configs_cli'; -export { runCheckFtrCodeOwnersCli } from './src/functional_test_runner/run_check_ftr_code_owners'; - export { runJest } from './src/jest/run'; export * from './src/kbn_archiver_cli'; diff --git a/packages/kbn-test/src/functional_test_runner/cli/code_owners.ts b/packages/kbn-test/src/functional_test_runner/cli/code_owners.ts new file mode 100644 index 0000000000000..62948bbfcde55 --- /dev/null +++ b/packages/kbn-test/src/functional_test_runner/cli/code_owners.ts @@ -0,0 +1,48 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import { run } from '@kbn/dev-cli-runner'; +import { createFailError } from '@kbn/dev-cli-errors'; +import { getRepoFiles } from '@kbn/get-repo-files'; +import { getCodeOwnersEntries } from '@kbn/code-owners'; +import ignore from 'ignore'; + +const TEST_DIRECTORIES = ['test', 'x-pack/test', 'x-pack/test_serverless']; + +export async function checkFTRCodeOwnersCLI() { + await run( + async ({ log }) => { + const matcher = ignore().add( + getCodeOwnersEntries() + .filter((entry) => entry.teams.length > 0) + .map((entry) => entry.pattern) + ); + const hasOwner = (path: string): boolean => matcher.test(path).ignored; + + const testFiles = await getRepoFiles(TEST_DIRECTORIES); + const filesWithoutOwner = testFiles + .filter((repoPath) => !hasOwner(repoPath.repoRel)) + .map((repoPath) => repoPath.repoRel); + + log.info(`Checked ${testFiles.length} test files in ${process.uptime().toFixed(2)}s`); + + if (filesWithoutOwner.length === 0) { + log.success(`All test files have a code owner 🥳`); + return; + } + + log.write('Test files without a code owner:'); + log.write(filesWithoutOwner.map((i) => ` - ${i}`).join('\n')); + throw createFailError(`Found ${filesWithoutOwner.length} test files without code owner`); + }, + { + description: 'Check that all test files are covered by GitHub CODEOWNERS', + } + ); +} diff --git a/packages/kbn-test/src/functional_test_runner/cli.ts b/packages/kbn-test/src/functional_test_runner/cli/ftr.ts similarity index 98% rename from packages/kbn-test/src/functional_test_runner/cli.ts rename to packages/kbn-test/src/functional_test_runner/cli/ftr.ts index fe153e17b8d69..d9694973e9252 100644 --- a/packages/kbn-test/src/functional_test_runner/cli.ts +++ b/packages/kbn-test/src/functional_test_runner/cli/ftr.ts @@ -16,8 +16,8 @@ import { ToolingLog } from '@kbn/tooling-log'; import { getTimeReporter } from '@kbn/ci-stats-reporter'; import exitHook from 'exit-hook'; -import { readConfigFile, EsVersion } from './lib'; -import { FunctionalTestRunner } from './functional_test_runner'; +import { readConfigFile, EsVersion } from '../lib'; +import { FunctionalTestRunner } from '../functional_test_runner'; export function runFtrCli() { const runStartTime = Date.now(); diff --git a/packages/kbn-test/src/functional_test_runner/cli/index.ts b/packages/kbn-test/src/functional_test_runner/cli/index.ts new file mode 100644 index 0000000000000..6e29c1bdc231f --- /dev/null +++ b/packages/kbn-test/src/functional_test_runner/cli/index.ts @@ -0,0 +1,11 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +export { runFtrCli } from './ftr'; +export { checkFTRCodeOwnersCLI } from './code_owners'; diff --git a/packages/kbn-test/src/functional_test_runner/index.ts b/packages/kbn-test/src/functional_test_runner/index.ts index 6c5641cbe8aab..42bdde23b0439 100644 --- a/packages/kbn-test/src/functional_test_runner/index.ts +++ b/packages/kbn-test/src/functional_test_runner/index.ts @@ -18,6 +18,6 @@ export { runCheckFtrConfigsCli, DedicatedTaskRunner, } from './lib'; -export { runFtrCli } from './cli'; +export * from './cli'; export * from './lib/docker_servers'; export * from './public_types'; diff --git a/packages/kbn-test/src/functional_test_runner/run_check_ftr_code_owners.ts b/packages/kbn-test/src/functional_test_runner/run_check_ftr_code_owners.ts deleted file mode 100644 index 8a1ec50bd1a3e..0000000000000 --- a/packages/kbn-test/src/functional_test_runner/run_check_ftr_code_owners.ts +++ /dev/null @@ -1,67 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the "Elastic License - * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side - * Public License v 1"; you may not use this file except in compliance with, at - * your election, the "Elastic License 2.0", the "GNU Affero General Public - * License v3.0 only", or the "Server Side Public License, v 1". - */ - -import { run } from '@kbn/dev-cli-runner'; -import { createFailError } from '@kbn/dev-cli-errors'; -import { getRepoFiles } from '@kbn/get-repo-files'; -import { getOwningTeamsForPath, getCodeOwnersEntries } from '@kbn/code-owners'; - -const TEST_DIRECTORIES = ['test', 'x-pack/test', 'x-pack/test_serverless']; - -const fmtMs = (ms: number) => { - if (ms < 1000) { - return `${Math.round(ms)} ms`; - } - - return `${(Math.round(ms) / 1000).toFixed(2)} s`; -}; - -const fmtList = (list: Iterable) => [...list].map((i) => ` - ${i}`).join('\n'); - -export async function runCheckFtrCodeOwnersCli() { - run( - async ({ log }) => { - const start = performance.now(); - - const missingOwners = new Set(); - - // cache codeowners for quicker lookup - log.info('Reading CODEOWNERS file'); - const codeOwnersEntries = getCodeOwnersEntries(); - - const testFiles = await getRepoFiles(TEST_DIRECTORIES); - log.info(`Checking ownership for ${testFiles.length} test files (this will take a while)`); - - for (const { repoRel } of testFiles) { - const owners = getOwningTeamsForPath(repoRel, codeOwnersEntries); - - if (owners.length === 0) { - missingOwners.add(repoRel); - } - } - - const timeSpent = fmtMs(performance.now() - start); - log.info(`Ownership check complete (took ${timeSpent})`); - - if (missingOwners.size) { - log.error( - `The following test files do not have a GitHub code owner:\n${fmtList(missingOwners)}` - ); - throw createFailError( - `Found ${missingOwners.size} test files without code owner (checked ${testFiles.length} test files in ${timeSpent})` - ); - } - - log.success(`All test files have a code owner. 🥳`); - }, - { - description: 'Check that all test files are covered by GitHub CODEOWNERS', - } - ); -} diff --git a/scripts/check_ftr_code_owners.js b/scripts/check_ftr_code_owners.js index 875f835d1d7b4..b35590500b313 100644 --- a/scripts/check_ftr_code_owners.js +++ b/scripts/check_ftr_code_owners.js @@ -8,4 +8,4 @@ */ require('../src/setup_node_env'); -require('@kbn/test').runCheckFtrCodeOwnersCli(); +void require('@kbn/test').checkFTRCodeOwnersCLI();