From 82a93fa8cdcb2e6c39976632985003c0bc6b7efe Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Thu, 12 May 2016 15:47:04 -0400 Subject: [PATCH] Labels: use "lib / src" when 5+ JS Subsystems --- lib/node-labels.js | 39 ++++++++++++++++++++++----- test/node-js-subsystem-labels.test.js | 6 ++--- test/node-labels.test.js | 18 ++++++++++--- 3 files changed, 50 insertions(+), 13 deletions(-) diff --git a/lib/node-labels.js b/lib/node-labels.js index 9512a08c..fda7f6d7 100644 --- a/lib/node-labels.js +++ b/lib/node-labels.js @@ -36,18 +36,25 @@ const subSystemLabelsMap = new Map([ [/^lib\/internal\/(\w+)$/, '$1'] ]) +const jsSubsystemList = [ + 'debugger', 'assert', 'buffer', 'child_process', 'cluster', 'console', + 'crypto', 'dgram', 'dns', 'domain', 'events', 'fs', 'http', 'https', 'module', + 'net', 'os', 'path', 'process', 'querystring', 'readline', 'repl', 'stream', + 'timers', 'tls', 'tty', 'url', 'util', 'v8', 'vm', 'zlib' +] + const exclusiveLabelsMap = new Map([ [/^test\//, 'test'], [/^doc\//, 'doc'], [/^benchmark\//, 'benchmark'] ]) -function resolveLabels (filepathsChanged) { +function resolveLabels (filepathsChanged, limitLib = true) { const exclusiveLabels = matchExclusiveSubSystem(filepathsChanged) return (exclusiveLabels.length > 0) ? exclusiveLabels - : matchAllSubSystem(filepathsChanged) + : matchAllSubSystem(filepathsChanged, limitLib) } function matchExclusiveSubSystem (filepathsChanged) { @@ -56,19 +63,37 @@ function matchExclusiveSubSystem (filepathsChanged) { return (isExclusive && labels.length === 1) ? labels : [] } -function matchAllSubSystem (filepathsChanged) { - return matchSubSystemsByRegex(subSystemLabelsMap, filepathsChanged) +function matchAllSubSystem (filepathsChanged, limitLib) { + return matchSubSystemsByRegex( + subSystemLabelsMap, filepathsChanged, limitLib) } -function matchSubSystemsByRegex (rxLabelsMap, filepathsChanged) { +function matchSubSystemsByRegex (rxLabelsMap, filepathsChanged, limitLib) { + const jsLabelCount = [] // by putting matched labels into a map, we avoid duplicate labels const labelsMap = filepathsChanged.reduce((map, filepath) => { const mappedSubSystem = mappedSubSystemForFile(rxLabelsMap, filepath) - if (mappedSubSystem) { - map[mappedSubSystem] = true + if (!mappedSubSystem) { + // short-circuit + return map + } + + if (limitLib && jsSubsystemList.includes(mappedSubSystem)) { + if (jsLabelCount.length >= 4) { + for (const jsLabel of jsLabelCount) { + delete map[jsLabel] + } + map['lib / src'] = true + // short-circuit + return map + } else { + jsLabelCount.push(mappedSubSystem) + } } + map[mappedSubSystem] = true + return map }, {}) diff --git a/test/node-js-subsystem-labels.test.js b/test/node-js-subsystem-labels.test.js index 7c534539..6a185815 100644 --- a/test/node-js-subsystem-labels.test.js +++ b/test/node-js-subsystem-labels.test.js @@ -35,7 +35,7 @@ tap.test('label: lib oddities', (t) => { 'lib/internal/v8_prof_processor.js' ] - const labels = nodeLabels.resolveLabels(libFiles) + const labels = nodeLabels.resolveLabels(libFiles, false) t.same(labels, [ 'debugger', // _debug_agent @@ -107,7 +107,7 @@ tap.test('label: lib normal without "lib / src" limiting', (t) => { 'lib/zlib.js' ] - const labels = nodeLabels.resolveLabels(libFiles) + const labels = nodeLabels.resolveLabels(libFiles, false) t.same(labels, libFiles.map((path) => /lib\/(_)?(\w+)\.js/.exec(path)[2])) @@ -126,7 +126,7 @@ tap.test('label: lib internals without "lib / src" limiting', (t) => { 'lib/internal/util.js' ] - const labels = nodeLabels.resolveLabels(libFiles) + const labels = nodeLabels.resolveLabels(libFiles, false) t.same(labels, libFiles.map((path) => /lib\/internal\/(\w+)\.js/.exec(path)[1])) diff --git a/test/node-labels.test.js b/test/node-labels.test.js index 35ce16b8..88259d19 100644 --- a/test/node-labels.test.js +++ b/test/node-labels.test.js @@ -133,17 +133,29 @@ tap.test('label: "repl" when ./lib/repl.js has been changed', (t) => { t.end() }) -tap.test('label: "lib / src" when more than 5 sub-systems has been changed', (t) => { +tap.test('label: "lib / src" when 5 or more JS sub-systems have been changed', (t) => { const labels = nodeLabels.resolveLabels([ 'lib/assert.js', 'lib/dns.js', 'lib/repl.js', 'lib/process.js', - 'src/async-wrap.cc', 'lib/module.js' ]) - t.same(labels, ['lib / src'], { todo: true }) + t.same(labels, ['lib / src']) + + t.end() +}) + +tap.test('label: "JS sub-systems when less than 5 sub-systems have changed', (t) => { + const labels = nodeLabels.resolveLabels([ + 'lib/assert.js', + 'lib/dns.js', + 'lib/repl.js', + 'lib/process.js' + ]) + + t.same(labels, ['assert', 'dns', 'repl', 'process']) t.end() })