From 45c411061dc238a6fb9f0cd589e089261add9bee Mon Sep 17 00:00:00 2001 From: Phillip Johnsen Date: Fri, 22 Apr 2016 22:55:15 +0200 Subject: [PATCH 1/3] Added `doc` and `benchmark` node core auto labels --- lib/node-labels.js | 35 ++++++++++++++++++++++------------- test/node-labels.test.js | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 13 deletions(-) diff --git a/lib/node-labels.js b/lib/node-labels.js index 2ab6079e..d4250fad 100644 --- a/lib/node-labels.js +++ b/lib/node-labels.js @@ -1,27 +1,36 @@ 'use strict' -const subSystemLabels = { +const subSystemLabelsMap = { 'c++': /^src\// } -const rxTests = /^test\// +const exclusiveLabelsMap = { + test: /^test\//, + doc: /^doc\//, + benchmark: /^benchmark\// +} function resolveLabels (filepathsChanged) { - if (isOnly(rxTests, filepathsChanged)) { - return ['test'] - } + const exclusiveLabels = matchExclusiveSubSystem(filepathsChanged) + + return (exclusiveLabels.length > 0) + ? exclusiveLabels + : matchAllSubSystem(filepathsChanged) +} - return matchBySubSystemRegex(filepathsChanged) +function matchExclusiveSubSystem (filepathsChanged) { + const labels = matchSubSystemsByRegex(exclusiveLabelsMap, filepathsChanged) + return labels.length === 1 ? labels : [] } -function isOnly (regexToMatch, filepathsChanged) { - return filepathsChanged.every((filepath) => regexToMatch.test(filepath)) +function matchAllSubSystem (filepathsChanged) { + return matchSubSystemsByRegex(subSystemLabelsMap, filepathsChanged) } -function matchBySubSystemRegex (filepathsChanged) { +function matchSubSystemsByRegex (rxLabelsMap, filepathsChanged) { // by putting matched labels into a map, we avoid duplicate labels const labelsMap = filepathsChanged.reduce((map, filepath) => { - const mappedSubSystem = mappedSubSystemForFile(filepath) + const mappedSubSystem = mappedSubSystemForFile(rxLabelsMap, filepath) if (mappedSubSystem) { map[mappedSubSystem] = true @@ -33,9 +42,9 @@ function matchBySubSystemRegex (filepathsChanged) { return Object.keys(labelsMap) } -function mappedSubSystemForFile (filepath) { - return Object.keys(subSystemLabels).find((labelName) => { - const rxForLabel = subSystemLabels[labelName] +function mappedSubSystemForFile (labelsMap, filepath) { + return Object.keys(labelsMap).find((labelName) => { + const rxForLabel = labelsMap[labelName] return rxForLabel.test(filepath) ? labelName : undefined }) diff --git a/test/node-labels.test.js b/test/node-labels.test.js index fe09a74c..2825fab8 100644 --- a/test/node-labels.test.js +++ b/test/node-labels.test.js @@ -16,6 +16,40 @@ tap.test('label: "test" when only ./test/ files has been changed', (t) => { t.end() }) +tap.test('no labels: 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.end() +}) + +tap.test('label: "doc" when only ./doc/ files has been changed', (t) => { + const labels = nodeLabels.resolveLabels([ + 'doc/api/fs.md', + 'doc/api/http.md', + 'doc/onboarding.md' + ]) + + t.same(labels, ['doc']) + + t.end() +}) + +tap.test('label: "benchmark" when only ./benchmark/ files has been changed', (t) => { + const labels = nodeLabels.resolveLabels([ + 'benchmark/http_server_lag.js', + 'benchmark/http/check_is_http_token.js' + ]) + + t.same(labels, ['benchmark']) + + t.end() +}) + tap.test('label: "c++" when ./src/* has been changed', (t) => { const labels = nodeLabels.resolveLabels([ 'src/async-wrap.h', From e8f518bab93592e921b55083bff1203e74fd6608 Mon Sep 17 00:00:00 2001 From: Phillip Johnsen Date: Fri, 22 Apr 2016 23:59:10 +0200 Subject: [PATCH 2/3] Added `deps/X` node core auto labels --- lib/node-labels.js | 32 ++++++++++++++++++++++++++++---- test/node-labels.test.js | 31 +++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/lib/node-labels.js b/lib/node-labels.js index d4250fad..ce8b4597 100644 --- a/lib/node-labels.js +++ b/lib/node-labels.js @@ -1,7 +1,12 @@ 'use strict' +// order of entries in this map *does* matter for the resolved labels const subSystemLabelsMap = { - 'c++': /^src\// + 'c++': /^src\//, + // libuv needs an explicit mapping, as the ordinary /deps/ mapping below would + // end up as libuv changes labeled with "uv" (which is a non-existing label) + 'libuv': /^deps\/uv\//, + '$1': /^deps\/([^/]+)/ } const exclusiveLabelsMap = { @@ -43,11 +48,30 @@ function matchSubSystemsByRegex (rxLabelsMap, filepathsChanged) { } function mappedSubSystemForFile (labelsMap, filepath) { - return Object.keys(labelsMap).find((labelName) => { + return Object.keys(labelsMap).map((labelName) => { const rxForLabel = labelsMap[labelName] + const matches = rxForLabel.exec(filepath) - return rxForLabel.test(filepath) ? labelName : undefined - }) + // return undefined when subsystem regex didn't match, + // we'll filter out these values with the .filter() below + if (matches === null) { + return undefined + } + + // label names starting with $ means we want to extract a matching + // group from the regex we've just matched against + if (labelName.startsWith('$')) { + const wantedMatchGroup = labelName.substr(1) + return matches[wantedMatchGroup] + } + + // use label name as is when label doesn't look like a regex matching group + return labelName + }).filter(withoutUndefinedValues)[0] +} + +function withoutUndefinedValues (label) { + return label !== undefined } exports.resolveLabels = resolveLabels diff --git a/test/node-labels.test.js b/test/node-labels.test.js index 2825fab8..665daabb 100644 --- a/test/node-labels.test.js +++ b/test/node-labels.test.js @@ -61,6 +61,37 @@ tap.test('label: "c++" when ./src/* has been changed', (t) => { t.end() }) +tap.test('label: "v8" when ./deps/v8/ files has been changed', (t) => { + const labels = nodeLabels.resolveLabels([ + 'deps/v8/src/arguments.cc' + ]) + + t.same(labels, ['v8']) + + t.end() +}) + +tap.test('label: "libuv" when ./deps/ub/ files has been changed', (t) => { + const labels = nodeLabels.resolveLabels([ + 'deps/uv/src/fs-poll.c' + ]) + + t.same(labels, ['libuv']) + + t.end() +}) + +tap.test('label: "v8", "openssl" when ./deps/v8/ and ./deps/openssl/ files has been changed', (t) => { + const labels = nodeLabels.resolveLabels([ + 'deps/v8/src/arguments.cc', + 'deps/openssl/openssl/ssl/ssl_rsa.c' + ]) + + t.same(labels, ['v8', 'openssl']) + + t.end() +}) + // // Planned tests to be resolved later // From 1219bc92947d829aa797d6dbe79df43550073748 Mon Sep 17 00:00:00 2001 From: Phillip Johnsen Date: Tue, 26 Apr 2016 08:06:48 +0200 Subject: [PATCH 3/3] Avoid c++ label when only src/node_version.h is affected --- lib/node-labels.js | 3 ++- test/node-labels.test.js | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/node-labels.js b/lib/node-labels.js index ce8b4597..b1a128c0 100644 --- a/lib/node-labels.js +++ b/lib/node-labels.js @@ -2,7 +2,8 @@ // order of entries in this map *does* matter for the resolved labels const subSystemLabelsMap = { - 'c++': /^src\//, + // don't want to label it a c++ update when we're "only" bumping the Node.js version + 'c++': /^src\/(?!node_version\.h)/, // libuv needs an explicit mapping, as the ordinary /deps/ mapping below would // end up as libuv changes labeled with "uv" (which is a non-existing label) 'libuv': /^deps\/uv\//, diff --git a/test/node-labels.test.js b/test/node-labels.test.js index 665daabb..ce8ab1a9 100644 --- a/test/node-labels.test.js +++ b/test/node-labels.test.js @@ -61,6 +61,16 @@ tap.test('label: "c++" when ./src/* has been changed', (t) => { t.end() }) +tap.test('label: not "c++" when ./src/node_version.h has been changed', (t) => { + const labels = nodeLabels.resolveLabels([ + 'src/node_version.h' + ]) + + t.same(labels, []) + + t.end() +}) + tap.test('label: "v8" when ./deps/v8/ files has been changed', (t) => { const labels = nodeLabels.resolveLabels([ 'deps/v8/src/arguments.cc'