Skip to content

Commit

Permalink
feat: add needs-ci label
Browse files Browse the repository at this point in the history
  • Loading branch information
aduh95 committed Feb 11, 2021
1 parent 37a9a63 commit 63519fe
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 62 deletions.
33 changes: 20 additions & 13 deletions lib/node-labels.js
Original file line number Diff line number Diff line change
@@ -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([
Expand Down Expand Up @@ -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++'],
Expand All @@ -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'],
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand Down
4 changes: 3 additions & 1 deletion test/unit/node-build-label.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
})
6 changes: 5 additions & 1 deletion test/unit/node-js-subsystem-labels.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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/
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand Down Expand Up @@ -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()
})
Loading

0 comments on commit 63519fe

Please sign in to comment.