From 8613b6c6d9485757f5f68ec07fb8a5d76acf110e Mon Sep 17 00:00:00 2001 From: Marco Ippolito Date: Wed, 18 Oct 2023 18:27:44 +0200 Subject: [PATCH] feat: mark and skip flaky test --- .gitignore | 1 + bin/ncu-ci.js | 28 +++-- docs/ncu-ci.md | 2 + lib/ci/ci_mark_flaky.js | 222 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 246 insertions(+), 7 deletions(-) create mode 100644 lib/ci/ci_mark_flaky.js diff --git a/.gitignore b/.gitignore index 7a46dfb4..3f1b5364 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,4 @@ tmp-* .eslintcache .ncu package-lock.json +.vscode diff --git a/bin/ncu-ci.js b/bin/ncu-ci.js index 2a1ba02e..eeeeb223 100755 --- a/bin/ncu-ci.js +++ b/bin/ncu-ci.js @@ -31,6 +31,7 @@ import auth from '../lib/auth.js'; import Request from '../lib/request.js'; import CLI from '../lib/cli.js'; import { hideBin } from 'yargs/helpers'; +import { markFlakyTests } from '../lib/ci/ci_mark_flaky.js'; setVerbosityFromEnv(); @@ -82,7 +83,7 @@ const args = yargs(hideBin(process.argv)) .option('cache', { default: false, describe: 'Cache the responses from Jenkins in .ncu/cache/ under' + - ' the node-core-utils installation directory' + ' the node-core-utils installation directory' }) .option('limit', { default: 99, @@ -91,13 +92,17 @@ const args = yargs(hideBin(process.argv)) .option('since ', { type: 'string', describe: 'Time since when the CI results should be queried' + }).option('mark-flaky', { + describe: 'Mark as flaky tests that have inconsistent failures', + type: 'boolean', + default: false }).check(argv => { try { // eslint-disable-next-line no-new new Date(argv.since); } catch { throw new Error('--since should be string that can ' + - 'be parsed by new Date()'); + 'be parsed by new Date()'); } return true; }); @@ -200,7 +205,7 @@ const args = yargs(hideBin(process.argv)) .option('cache', { default: false, describe: 'Cache the responses from Jenkins in .ncu/cache/ under' + - ' the node-core-utils installation directory' + ' the node-core-utils installation directory' }) .option('limit', { default: 15, @@ -227,6 +232,10 @@ const args = yargs(hideBin(process.argv)) .option('markdown ', { type: 'string', describe: 'Write the results as markdown to ' + }).option('mark-flaky', { + describe: 'Mark as flaky tests that have inconsistent failures', + type: 'boolean', + default: false }).check(argv => { if (argv.markdown && commandKeys.includes(argv.markdown)) { throw new Error('--markdown did not specify a valid path'); @@ -275,11 +284,11 @@ class RunPRJobCommand { if (!repo) { validArgs = false; cli.error('GitHub repository is missing, please set it via ncu-config ' + - 'or pass it via the --repo option'); + 'or pass it via the --repo option'); } if (!owner) { cli.error('GitHub owner is missing, please set it via ncu-config ' + - 'or pass it via the --owner option'); + 'or pass it via the --owner option'); validArgs = false; } if (!validArgs) { @@ -379,7 +388,7 @@ class CICommand { } } - async aggregate() {} // noop + async aggregate() { } // noop async serialize() { const { argv, cli } = this; @@ -428,6 +437,7 @@ class RateCommand extends CICommand { class WalkCommand extends CICommand { constructor(cli, request, argv) { super(cli, request, argv); + this.markFlaky = argv.markFlaky; if (argv.cache) { jobCache.enable(); } @@ -453,7 +463,11 @@ class WalkCommand extends CICommand { return; } const aggregator = new FailureAggregator(cli, this.json); - this.json = aggregator.aggregate(); + const aggregation = aggregator.aggregate(); + if (this.markFlaky) { + await markFlakyTests(aggregation); + } + this.json = aggregation; cli.log(''); cli.separator('Stats'); cli.log(''); diff --git a/docs/ncu-ci.md b/docs/ncu-ci.md index ecea67b9..8a214755 100644 --- a/docs/ncu-ci.md +++ b/docs/ncu-ci.md @@ -32,6 +32,8 @@ Options: [boolean] [default: false] --json Write the results as json to [string] --markdown Write the results as markdown to [string] + --mark-flaky If running walk, whether or not mark tests as flaky. + [boolean] [default: false] --help Show help [boolean] ``` diff --git a/lib/ci/ci_mark_flaky.js b/lib/ci/ci_mark_flaky.js new file mode 100644 index 00000000..68da50a6 --- /dev/null +++ b/lib/ci/ci_mark_flaky.js @@ -0,0 +1,222 @@ +import _ from 'lodash'; +import { createWriteStream } from 'node:fs'; +import { appendFile, open, rename } from 'node:fs/promises'; +import { Transform } from 'node:stream'; +import { pipeline } from 'node:stream/promises'; + +export async function markFlakyTests(aggregation) { + try { + const tests = getFlakyTests(aggregation); + // group tests by type (ex: parallel, pummel) + const groupedByType = _.groupBy(tests, ({ file }) => file.split('/', 1)); + + for (const [type, failedTests] of Object.entries(groupedByType)) { + await editStatusFile(type, failedTests); + } + } catch (error) { + console.error(error); + } +}; + +export function getFlakyTests(aggregation) { + const failedRuns = []; + const { JS_TEST_FAILURE } = aggregation; + + for (const failedTest of JS_TEST_FAILURE) { + const { failures } = failedTest; + if (!failures) continue; + for (const failure of failures) { + const { builtOn, file } = failure; + if (!builtOn) continue; + const { system, architecture } = parseSystemArchitecture(builtOn); + failedRuns.push({ + builtOn, + file, + system, + architecture, + written: false + }); + } + } + + return failedRuns; +} + +function matchSystem(rawSystem) { + let system; + switch (true) { + case rawSystem.includes('container'): + system = 'docker'; + break; + case rawSystem.includes('win'): + system = 'win32'; + break; + case rawSystem.includes('fedora'): + case rawSystem.includes('ubuntu'): + case rawSystem.includes('rhel'): + case rawSystem.includes('debian'): + system = 'linux'; + break; + case rawSystem.includes('macos'): + system = 'macos'; + break; + case rawSystem.includes('solaris'): + case rawSystem.includes('smartos'): + system = 'solaris'; + break; + case rawSystem.includes('freebsd'): + system = 'freebsd'; + break; + case rawSystem.includes('aix72'): + system = 'aix'; + break; + default: + system = rawSystem; + break; + } + + return system; +} + +function matchArchitecture(rawArchitecture) { + let architecture; + switch (true) { + case rawArchitecture.includes('arm64'): + architecture = 'arm64'; + break; + case rawArchitecture.includes('arm'): + architecture = 'arm'; + break; + case rawArchitecture.includes('s390x'): + case rawArchitecture.includes('ppc64'): + architecture = 'ibm'; + break; + default: + architecture = rawArchitecture; + break; + } + return architecture; +} + +function parseSystemArchitecture(builtOn) { + const buildInfos = builtOn.split('-'); + const rawArchitecture = buildInfos[buildInfos.length - 2]; // second last element is architecture + const rawSystem = buildInfos[buildInfos.length - 3]; // third last element is os + + return { + architecture: matchArchitecture(rawArchitecture), + system: matchSystem(rawSystem) + }; +} + +async function editStatusFile(type, failedTests) { + try { + const groupedByHeaders = _.groupBy(failedTests, (f) => createHeader(f.system, f.architecture)); + // assume the .status file exists + const fileName = `./test/${type}/${type}.status`; + const tmpFile = `${fileName}.tmp`; + const file = await open(fileName); + + await pipeline( + file.readLines(), + new FlakyTestTransfrom(groupedByHeaders), + createWriteStream(tmpFile) // write output on a temp file + ); + + // if the header was not present we append it at the end of the file + await appendTestsWithNewHeader(tmpFile, groupedByHeaders); + + await rename(tmpFile, fileName); + } catch (error) { + // file might not exist or error was not parsable + console.error(error); + } +} + +function createHeader(system, architecture) { + return `[$system==${system} && $arch==${architecture}]`; +} + +function generateSkipFileLine(file) { + // take only filename without ex: /parallel/ + const filename = file.split('/')[1]; + return `${filename}: PASS, FLAKY`; +} + +function appendTestsWithNewHeader(tmpFile, groupedByHeaders) { + const text = []; + + for (const [header, failedTests] of Object.entries(groupedByHeaders)) { + // skip if there isnt at least one failedTest with written false or no failedTests + if (!failedTests?.length || !failedTests.some(f => f.written === false)) continue; + + // add space on top of header + text.push('\n' + header); + + // add test lines in a set to avoid duplicates + const newLines = new Set(); + for (const failedTest of failedTests) { + // skip tests we have already been written because we found the header + if (failedTest.written) continue; + newLines.add(generateSkipFileLine(failedTest.file)); + } + text.push(...newLines); + } + + return appendFile(tmpFile, text.join('\n')); +} + +class FlakyTestTransfrom extends Transform { + constructor(groupedByHeaders) { + super(); + this.groupedByHeaders = groupedByHeaders; + this.bufferedLines = []; + this.uniqueTests = new Set(); + } + + _transform(chunk, _encoding, callback) { + const chunkStringified = chunk.toString(); + + // keys of groupedByHeaders are [$system==win32 && $arch==arm64] + const failedTests = this.groupedByHeaders[chunkStringified]; + + if (failedTests?.length) { + // when we hit a new header, push bufferedLines + this.push(this.bufferedLinesToString()); + + // reset + this.bufferedLines = []; + this.uniqueTests = new Set(); + + // header first + this.bufferedLines.push(chunkStringified); + + for (const failedTest of failedTests) { + // set written to true because there was an existing header + failedTest.written = true; + const skipFileLine = generateSkipFileLine(failedTest.file); + this.bufferedLines.push(skipFileLine); + this.uniqueTests.add(skipFileLine); + } + // this is a non header line + // we check if its a test we have added already + } else if (!this.uniqueTests.has(chunkStringified)) { + this.bufferedLines.push(chunkStringified); + } + + callback(); + } + + bufferedLinesToString() { + if (this.bufferedLines.length > 0) { + return this.bufferedLines.join('\n') + '\n'; + } + return '\n'; + } + + _flush(callback) { + // Push any remaining buffered lines + this.push(this.bufferedLinesToString()); + callback(); + } +}