Labels: use "lib / src" when 5+ JS Subsystems

nodejs · May 12, 2016 · 35657c0 · 35657c0
1 parent 60faaf4
commit 35657c0
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 13 deletions.
diff --git a/lib/node-labels.js b/lib/node-labels.js
@@ -33,18 +33,25 @@ const subSystemLabelsMap = new Map([
   [/^lib\/internal\/(\w+)$/, '$1'] // internal subfolders
 ])
 
+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) {
@@ -53,19 +60,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
@@ -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
@@ -106,7 +106,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]))
 
@@ -125,7 +125,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
@@ -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()
 })