From 737094cf07224be73eafa279356fd44a1cc55c87 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Thu, 11 Feb 2021 19:00:12 +0100 Subject: [PATCH] feat: add `needs-ci` label --- lib/node-labels.js | 33 +++++--- test/unit/node-build-label.test.js | 4 +- test/unit/node-js-subsystem-labels.test.js | 6 +- test/unit/node-labels.test.js | 94 +++++++++++----------- 4 files changed, 75 insertions(+), 62 deletions(-) diff --git a/lib/node-labels.js b/lib/node-labels.js index c8ef0748..cc3a3ee6 100644 --- a/lib/node-labels.js +++ b/lib/node-labels.js @@ -1,5 +1,6 @@ 'use strict' +const ciNeededFolderRx = /^(deps|lib|src|test)\// // order of entries in this map *does* matter for the resolved labels // earlier entries override later entries const subSystemLabelsMap = new Map([ @@ -37,10 +38,10 @@ const subSystemLabelsMap = new Map([ [/^src\/node_bob*/, ['c++', 'quic', 'dont-land-on-v14.x', 'dont-land-on-v12.x']], // don't label python files as c++ - [/^src\/.+\.py$/, 'lib / src'], + [/^src\/.+\.py$/, ['lib / src', 'needs-ci']], // properly label changes to v8 inspector integration-related files - [/^src\/inspector_/, ['c++', 'inspector']], + [/^src\/inspector_/, ['c++', 'inspector', 'needs-ci']], // don't want to label it a c++ update when we're "only" bumping the Node.js version [/^src\/(?!node_version\.h)/, 'c++'], @@ -51,22 +52,24 @@ const subSystemLabelsMap = new Map([ // things that edit top-level .md files are always a doc change [/^\w+\.md$/, 'doc'], // different variants of *Makefile and build files - [/^(tools\/)?(Makefile|BSDmakefile|create_android_makefiles|\.travis\.yml)$/, 'build'], - [/^tools\/(install\.py|genv8constants\.py|getnodeversion\.py|js2c\.py|utils\.py|configure\.d\/.*)$/, 'build'], - [/^vcbuild\.bat$/, ['build', 'windows']], - [/^(android-)?configure|node\.gyp|common\.gypi$/, 'build'], + [/^(tools\/)?(Makefile|BSDmakefile|create_android_makefiles|\.travis\.yml)$/, ['build', 'needs-ci']], + [/^tools\/(install\.py|genv8constants\.py|getnodeversion\.py|js2c\.py|utils\.py|configure\.d\/.*)$/, ['build', 'needs-ci']], + [/^vcbuild\.bat$/, ['build', 'windows', 'needs-ci']], + [/^(android-)?configure|node\.gyp|common\.gypi$/, ['build', 'needs-ci']], // more specific tools - [/^tools\/gyp/, ['tools', 'build']], + [/^tools\/gyp/, ['tools', 'build', 'needs-ci']], [/^tools\/doc\//, ['tools', 'doc']], - [/^tools\/icu\//, ['tools', 'intl']], + [/^tools\/icu\//, ['tools', 'intl', 'needs-ci']], [/^tools\/(?:osx-pkg\.pmdoc|pkgsrc)\//, ['tools', 'macos', 'install']], [/^tools\/(?:(?:mac)?osx-)/, ['tools', 'macos']], [/^tools\/test-npm/, ['tools', 'test', 'npm']], [/^tools\/test/, ['tools', 'test']], [/^tools\/(?:certdata|mkssldef|mk-ca-bundle)/, ['tools', 'openssl', 'tls']], - [/^tools\/msvs\//, ['tools', 'windows', 'install']], - [/^tools\/[^/]+\.bat$/, ['tools', 'windows']], - [/^tools\/make-v8/, ['tools', 'V8 Engine']], + [/^tools\/msvs\//, ['tools', 'windows', 'install', 'needs-ci']], + [/^tools\/[^/]+\.bat$/, ['tools', 'windows', 'needs-ci']], + [/^tools\/make-v8/, ['tools', 'V8 Engine', 'needs-ci']], + [/^tools\/(code_cache|snapshot|v8_gypfiles)/, 'needs-ci'], + [/^tools\/build-addons.js/, 'needs-ci'], // all other tool changes should be marked as such [/^tools\//, 'tools'], [/^\.eslint|\.remark|\.editorconfig/, 'tools'], @@ -239,6 +242,10 @@ function matchSubSystemsByRegex (rxLabelsMap, filepathsChanged, limitLabels) { const labelCount = [] // by putting matched labels into a map, we avoid duplicate labels const labelsMap = filepathsChanged.reduce((map, filepath) => { + if (ciNeededFolderRx.test(filepath) && !map['needs-ci']) { + map['needs-ci'] = true + } + const mappedSubSystems = mappedSubSystemsForFile(rxLabelsMap, filepath) if (!mappedSubSystems) { @@ -251,8 +258,8 @@ function matchSubSystemsByRegex (rxLabelsMap, filepathsChanged, limitLabels) { if (limitLabels && hasLibOrSrcChanges(filepathsChanged)) { if (labelCount.length >= 4) { for (const label of labelCount) { - // don't delete the c++ label as we always want that if it has matched - if (label !== 'c++') delete map[label] + // don't delete the `c++` or `needs-ci` labels as we always want those if they have matched + if (label !== 'c++' && label !== 'needs-ci') delete map[label] } map['lib / src'] = true // short-circuit diff --git a/test/unit/node-build-label.test.js b/test/unit/node-build-label.test.js index da0b4746..c159dab5 100644 --- a/test/unit/node-build-label.test.js +++ b/test/unit/node-build-label.test.js @@ -24,7 +24,8 @@ tap.test('label: "build" when build related files has been changed', (t) => { buildRelatedFiles.forEach((filepath) => { const labels = nodeLabels.resolveLabels([ filepath ]) - t.same(labels, ['build'], filepath + ' got "build" label') + t.ok(labels.includes('build'), filepath + ' should have "build" label') + t.ok(labels.includes('needs-ci'), filepath + ' should have "needs-ci" label') }) t.end() @@ -36,6 +37,7 @@ tap.test('labels: not "build" when Makefile in ./deps has been changed', (t) => ]) t.notOk(labels.includes('build')) + t.ok(labels.includes('needs-ci')) t.end() }) diff --git a/test/unit/node-js-subsystem-labels.test.js b/test/unit/node-js-subsystem-labels.test.js index dc9b51d3..b2ac8df0 100644 --- a/test/unit/node-js-subsystem-labels.test.js +++ b/test/unit/node-js-subsystem-labels.test.js @@ -38,6 +38,7 @@ tap.test('label: lib oddities', (t) => { const labels = nodeLabels.resolveLabels(libFiles, false) t.same(labels, [ + 'needs-ci', // lib/ 'debugger', // _debug_agent 'http', // _http_* 'timers', // linklist @@ -62,6 +63,7 @@ tap.test('label: lib internals oddities duplicates', (t) => { const labels = nodeLabels.resolveLabels(libFiles) t.same(labels, [ + 'needs-ci', // lib/ 'lib / src', // bootstrap_node 'timers', // linkedlist 'stream' // internal/streams/ @@ -108,6 +110,7 @@ tap.test('label: lib normal without "lib / src" limiting', (t) => { const labels = nodeLabels.resolveLabels(libFiles, false) + t.same(labels.shift(), 'needs-ci') t.same(labels, libFiles.map((path) => /lib\/(_)?(\w+)\.js/.exec(path)[2])) t.end() @@ -127,6 +130,7 @@ tap.test('label: lib internals without "lib / src" limiting', (t) => { const labels = nodeLabels.resolveLabels(libFiles, false) + t.same(labels.shift(), 'needs-ci') t.same(labels, libFiles.map((path) => /lib\/internal\/(\w+)\.js/.exec(path)[1])) t.end() @@ -169,7 +173,7 @@ tap.test('label: appropriate labels for files in internal subdirectories', (t) = 'lib/internal/process/next_tick.js' ], false) - t.same(labels, ['cluster', 'process']) + t.same(labels, ['needs-ci', 'cluster', 'process']) t.end() }) diff --git a/test/unit/node-labels.test.js b/test/unit/node-labels.test.js index 33ff3447..73ed28f8 100644 --- a/test/unit/node-labels.test.js +++ b/test/unit/node-labels.test.js @@ -4,13 +4,13 @@ const tap = require('tap') const nodeLabels = require('../../lib/node-labels') -tap.test('no labels: when ./test/ and ./doc/ files has been changed', (t) => { +tap.test('label: "needs-ci" when ./test/ and ./doc/ files has been changed', (t) => { const labels = nodeLabels.resolveLabels([ 'test/debugger/test-debugger-pid.js', 'doc/api/fs.md' ]) - t.same(labels, []) + t.same(labels, ['needs-ci']) t.end() }) @@ -18,13 +18,13 @@ tap.test('no labels: when ./test/ and ./doc/ files has been changed', (t) => { // This ensures older mislabelling issues doesn't happen again // https://github.com/nodejs/node/pull/6432 // https://github.com/nodejs/node/pull/6448 -tap.test('no labels: when ./test/ and ./lib/ files has been changed', (t) => { +tap.test('label: "needs-ci" when ./test/ and ./lib/ files has been changed', (t) => { const labels = nodeLabels.resolveLabels([ 'lib/punycode.js', 'test/parallel/test-assert.js' ]) - t.same(labels, []) + t.same(labels, ['needs-ci']) t.end() }) @@ -66,7 +66,7 @@ tap.test('label: "c++" when ./src/* has been changed', (t) => { 'src/node.cc' ]) - t.same(labels, ['c++']) + t.same(labels, ['needs-ci', 'c++']) t.end() }) @@ -154,7 +154,7 @@ for (const info of srcCases) { tap.test(`label: "${labels.join('","')}" when ./src/${file} has been changed`, (t) => { const resolved = nodeLabels.resolveLabels([`src/${file}`]) - t.same(resolved, ['c++'].concat(labels)) + t.same(resolved, ['needs-ci', 'c++'].concat(labels)) t.end() }) @@ -166,7 +166,7 @@ tap.test('label: not "c++" when ./src/node_version.h has been changed', (t) => { 'src/node_version.h' ]) - t.same(labels, []) + t.same(labels, ['needs-ci']) t.end() }) @@ -178,7 +178,7 @@ tap.test('label: not "c++" when ./src/*.py has been changed', (t) => { 'src/perfctr_macros.py' ]) - t.same(labels, ['lib / src']) + t.same(labels, ['needs-ci', 'lib / src']) t.end() }) @@ -188,7 +188,7 @@ tap.test('label: "inspector" when ./src/inspector_* has been changed', (t) => { 'src/inspector_socket.cc' ]) - t.same(labels, ['c++', 'inspector']) + t.same(labels, ['needs-ci', 'c++', 'inspector']) t.end() }) @@ -198,7 +198,7 @@ tap.test('label: "V8 Engine" when ./deps/v8/ files has been changed', (t) => { 'deps/v8/src/arguments.cc' ]) - t.same(labels, ['V8 Engine']) + t.same(labels, ['needs-ci', 'V8 Engine']) t.end() }) @@ -208,7 +208,7 @@ tap.test('label: "libuv" when ./deps/uv/ files has been changed', (t) => { 'deps/uv/src/fs-poll.c' ]) - t.same(labels, ['libuv']) + t.same(labels, ['needs-ci', 'libuv']) t.end() }) @@ -218,7 +218,7 @@ tap.test('label: "wasi" when ./deps/uvwasi/ files has been changed', (t) => { 'deps/uvwasi/src/uvwasi.c' ]) - t.same(labels, ['wasi']) + t.same(labels, ['needs-ci', 'wasi']) t.end() }) @@ -229,7 +229,7 @@ tap.test('label: "V8 Engine", "openssl" when ./deps/v8/ and ./deps/openssl/ file 'deps/openssl/openssl/ssl/ssl_rsa.c' ]) - t.same(labels, ['V8 Engine', 'openssl']) + t.same(labels, ['needs-ci', 'V8 Engine', 'openssl']) t.end() }) @@ -246,7 +246,7 @@ tap.test('label: "repl" when ./lib/repl.js has been changed', (t) => { 'test/debugger/test-debugger-repl-term.js' ]) - t.same(labels, ['repl']) + t.same(labels, ['needs-ci', 'repl']) t.end() }) @@ -260,7 +260,7 @@ tap.test('label: "lib / src" when 4 or more JS sub-systems have been changed', ( 'lib/module.js' ]) - t.same(labels, ['lib / src']) + t.same(labels, ['needs-ci', 'lib / src']) t.end() }) @@ -293,7 +293,7 @@ tap.test('label: "lib / src" when 4 or more native files have been changed', (t) 'src/uv.cc' ]) - t.same(labels, ['c++', 'lib / src']) + t.same(labels, ['needs-ci', 'c++', 'lib / src']) t.end() }) @@ -308,7 +308,7 @@ tap.test('label: not "lib / src" when only deps have been changed', (t) => { 'deps/v8/test/cctest/interpreter/bytecode_expectations/BasicLoops.golden' ]) - t.same(labels, ['V8 Engine']) + t.same(labels, ['needs-ci', 'V8 Engine']) t.end() }) @@ -321,7 +321,7 @@ tap.test('label: "JS sub-systems when less than 4 sub-systems have changed', (t) 'lib/process.js' ]) - t.same(labels, ['assert', 'dns', 'repl', 'process']) + t.same(labels, ['needs-ci', 'assert', 'dns', 'repl', 'process']) t.end() }) @@ -388,7 +388,7 @@ tap.test('label: version labels (old)', (t) => { 'common.gypi' ], 'v0.12') - t.same(labels, ['build', 'v0.12']) + t.same(labels, ['build', 'needs-ci', 'v0.12']) t.end() }) @@ -398,7 +398,7 @@ tap.test('label: version labels (old, staging)', (t) => { 'common.gypi' ], 'v0.12-staging') - t.same(labels, ['build', 'v0.12']) + t.same(labels, ['build', 'needs-ci', 'v0.12']) t.end() }) @@ -410,7 +410,7 @@ tap.test('label: version labels (new)', (t) => { 'deps/v8/test/mjsunit/regress/regress-5033.js' ], 'v6.x') - t.same(labels, ['V8 Engine', 'v6.x']) + t.same(labels, ['needs-ci', 'V8 Engine', 'v6.x']) t.end() }) @@ -422,7 +422,7 @@ tap.test('label: version labels (new, staging)', (t) => { 'deps/v8/test/mjsunit/regress/regress-5033.js' ], 'v6.x-staging') - t.same(labels, ['V8 Engine', 'v6.x']) + t.same(labels, ['needs-ci', 'V8 Engine', 'v6.x']) t.end() }) @@ -434,7 +434,7 @@ tap.test('label: no version labels (master)', (t) => { 'deps/v8/test/mjsunit/regress/regress-5033.js' ], 'master') - t.same(labels, ['V8 Engine']) + t.same(labels, ['needs-ci', 'V8 Engine']) t.end() }) @@ -444,7 +444,7 @@ tap.test('label: build label (windows)', (t) => { 'vcbuild.bat' ]) - t.same(labels, ['build', 'windows']) + t.same(labels, ['build', 'windows', 'needs-ci']) t.end() }) @@ -532,9 +532,9 @@ const specificTests = [ for (const info of specificTests) { let labels = info[0] if (!Array.isArray(labels)) { - labels = ['test', labels] + labels = ['needs-ci', 'test', labels] } else { - labels = ['test'].concat(labels) + labels = ['needs-ci', 'test'].concat(labels) } const files = info[1] for (const file of files) { @@ -549,9 +549,9 @@ for (const info of specificTests) { } const specificTools = [ - ['build', ['gyp/gyp_main.py', 'gyp_node.py']], + [['build', 'needs-ci'], ['gyp/gyp_main.py', 'gyp_node.py']], ['doc', ['doc/generate.js']], - ['intl', ['icu/icu-generate.gyp']], + [['intl', 'needs-ci'], ['icu/icu-generate.gyp']], ['macos', ['macosx-firewall.sh', 'osx-codesign.sh']], @@ -561,9 +561,9 @@ const specificTools = [ [['test', 'npm'], ['test-npm.sh', 'test-npm-package.js']], [['test'], ['test.py']], [['openssl', 'tls'], ['certdata.txt', 'mkssldef.py', 'mk-ca-bundle.pl']], - [['windows'], ['sign.bat']], - [['windows', 'install'], ['msvs/msi/product.wxs']], - [['V8 Engine'], ['make-v8.sh']] + [['windows', 'needs-ci'], ['sign.bat']], + [['windows', 'install', 'needs-ci'], ['msvs/msi/product.wxs']], + [['V8 Engine', 'needs-ci'], ['make-v8.sh']] ] for (const info of specificTools) { let labels = info[0] @@ -585,11 +585,11 @@ for (const info of specificTools) { } [ - [['V8 Engine', 'post-mortem'], + [['needs-ci', 'V8 Engine', 'post-mortem'], ['deps/v8/tools/gen-postmortem-metadata.py']], - [['c++', 'n-api'], + [['needs-ci', 'c++', 'n-api'], ['src/node_api.cc', 'src/node_api.h', 'src/node_api_types.h']], - [['test', 'n-api'], + [['needs-ci', 'test', 'n-api'], ['test/addons-napi/foo']], [['doc', 'n-api'], ['doc/api/n-api.md']] @@ -608,8 +608,8 @@ for (const info of specificTools) { }); [ - [['async_hooks'], ['lib/async_hooks.js']], - [['test', 'async_hooks'], ['test/async-hooks/test-connection.ssl.js']] + [['needs-ci', 'async_hooks'], ['lib/async_hooks.js']], + [['needs-ci', 'test', 'async_hooks'], ['test/async-hooks/test-connection.ssl.js']] ].forEach((info) => { const labels = info[0] const files = info[1] @@ -629,7 +629,7 @@ tap.test('label: "build" when ./android-configure has been changed', (t) => { 'android-configure' ]) - t.same(labels, ['build']) + t.same(labels, ['build', 'needs-ci']) t.end() }) @@ -639,22 +639,22 @@ tap.test('label: "build" when ./.travis.yml has been changed', (t) => { '.travis.yml' ]) - t.same(labels, ['build']) + t.same(labels, ['build', 'needs-ci']) t.end() }); [ - [['http2', 'dont-land-on-v6.x'], + [['needs-ci', 'http2', 'dont-land-on-v6.x'], ['lib/http2.js', 'lib/internal/http2/core.js', 'deps/nghttp2/lib/nghttp2_buf.c']], - [['c++', 'http2', 'dont-land-on-v6.x'], + [['needs-ci', 'c++', 'http2', 'dont-land-on-v6.x'], ['src/node_http2.cc', 'src/node_http2.h', 'src/node_http2_core.h', 'src/node_http2_core-inl.h']], - [['build', 'http2', 'dont-land-on-v6.x'], + [['needs-ci', 'build', 'http2', 'dont-land-on-v6.x'], ['deps/nghttp2/nghttp2.gyp']], [['doc', 'http2'], ['doc/api/http2.md']] ].forEach((info) => { @@ -672,13 +672,13 @@ tap.test('label: "build" when ./.travis.yml has been changed', (t) => { }); [ - [['c++', 'report'], + [['needs-ci', 'c++', 'report'], ['src/node_report.cc', 'src/node_report.h', 'src/node_report_module.cc', 'src/node_report_utils.cc']], [['doc', 'report'], ['doc/api/report.md']], - [['test', 'report'], ['test/report/test-report-config.js']] + [['needs-ci', 'test', 'report'], ['test/report/test-report-config.js']] ].forEach((info) => { const labels = info[0] const files = info[1] @@ -695,19 +695,19 @@ tap.test('label: "build" when ./.travis.yml has been changed', (t) => { [ // wasi - [['wasi'], + [['needs-ci', 'wasi'], ['lib/wasi.js']], - [['c++', 'wasi'], + [['needs-ci', 'c++', 'wasi'], ['src/node_wasi.cc', 'src/node_wasi.h']], [['doc', 'wasi'], ['doc/api/wasi.md']], // worker - [['worker'], + [['needs-ci', 'worker'], ['lib/worker_threads.js', 'lib/internal/worker.js', 'lib/internal/worker/io.js']], - [['c++', 'worker'], + [['needs-ci', 'c++', 'worker'], ['src/node_worker.cc', 'src/node_worker.h']], [['doc', 'worker'], ['doc/api/worker_threads.md']]