Skip to content

Commit

Permalink
Add JS (lib/) subsystem labels (#43)
Browse files Browse the repository at this point in the history 
* Labels: add JS Subsystem labels

* Labels: use "lib / src" when 5+ JS Subsystems
  • Loading branch information
@Fishrock123 @phillipj
Fishrock123 authored and phillipj committed May 13, 2016
1 parent ef93782 commit d8f0a38
Show file tree
Hide file tree
Showing 3 changed files with 201 additions and 13 deletions.
59 changes: 51 additions & 8 deletions lib/node-labels.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,49 @@ const subSystemLabelsMap = new Map([
[/^([A-Z]+$|CODE_OF_CONDUCT|ROADMAP|WORKING_GROUPS|GOVERNANCE|CHANGELOG|\.mail|\.git.+)/, 'meta'],
// things that edit top-level .md files are always a doc change
[/^\w+\.md$/, 'doc'],

/* Dependencies */
// 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)
[/^deps\/uv\//, 'libuv'],
[/^deps\/([^/]+)/, '$1']
[/^deps\/([^/]+)/, '$1'],

/* JS subsystems */
// Oddities first
[/^lib\/(punycode|\w+\/freelist|sys\.js)/, ''], // TODO: ignore better?
[/^lib\/string_decoder/, 'buffer'],
[/^lib\/constants\.js$/, 'lib / src'],
[/^lib\/_(debug_agent|debugger)\.js$/, 'debugger'],
[/^lib(\/\w+)?\/(_)?link(ed)?list/, 'timers'],
[/^lib\/\w+\/bootstrap_node/, 'lib / src'],
[/^lib\/\w+\/v8_prof_/, 'tools'],
[/^lib\/\w+\/socket_list/, 'net'],
[/^lib\/\w+\/streams$/, 'stream'],
// All other lib/ files map directly
[/^lib\/_(\w+)_\w+\.js?$/, '$1'], // e.g. _(stream)_wrap
[/^lib(\/internal)?\/(\w+)\.js?$/, '$2'], // other .js files
[/^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) {
Expand All @@ -35,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
}, {})

Expand Down
133 changes: 133 additions & 0 deletions test/node-js-subsystem-labels.test.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
'use strict'

const tap = require('tap')

const nodeLabels = require('../lib/node-labels')

tap.test('label: lib oddities', (t) => {
const libFiles = [
'lib/_debug_agent.js',
'lib/_http_agent.js',
'lib/_http_client.js',
'lib/_http_common.js',
'lib/_http_incoming.js',
'lib/_http_outgoing.js',
'lib/_http_server.js',
'lib/_linklist.js',
'lib/_stream_duplex.js',
'lib/_stream_passthrough.js',
'lib/_stream_readable.js',
'lib/_stream_transform.js',
'lib/_stream_wrap.js',
'lib/_stream_writable.js',
'lib/_tls_common.js',
'lib/_tls_legacy.js',
'lib/_tls_wrap.js',
'lib/constants.js',
'lib/punycode.js', // ignored
'lib/string_decoder.js',
'lib/sys.js', // ignored
'lib/internal/freelist.js', // ignored
'lib/internal/process',
'lib/internal/readme.md', // ignored
'lib/internal/socket_list.js',
'lib/internal/v8_prof_polyfill.js',
'lib/internal/v8_prof_processor.js'
]

const labels = nodeLabels.resolveLabels(libFiles, false)

t.same(labels, [
'debugger', // _debug_agent
'http', // _http_*
'timers', // linklist
'stream', // _stream_*
'tls', // _tls_*
'lib / src', // constants
'buffer', // string_decoder
'process', // internal/process/
'net', // socket_list
'tools' // v8_prof_*
])

t.end()
})

tap.test('label: lib internals oddities duplicates', (t) => {
const libFiles = [
'lib/internal/bootstrap_node.js',
'lib/internal/linkedlist.js',
'lib/internal/streams'
]

const labels = nodeLabels.resolveLabels(libFiles)

t.same(labels, [
'lib / src', // bootstrap_node
'timers', // linkedlist
'stream' // internal/streams/
])

t.end()
})

tap.test('label: lib normal without "lib / src" limiting', (t) => {
const libFiles = [
'lib/_debugger.js',
'lib/assert.js',
'lib/buffer.js',
'lib/child_process.js',
'lib/cluster.js',
'lib/console.js',
'lib/crypto.js',
'lib/dgram.js',
'lib/dns.js',
'lib/domain.js',
'lib/events.js',
'lib/fs.js',
'lib/http.js',
'lib/https.js',
'lib/module.js',
'lib/net.js',
'lib/os.js',
'lib/path.js',
'lib/process.js',
'lib/querystring.js',
'lib/readline.js',
'lib/repl.js',
'lib/stream.js',
'lib/timers.js',
'lib/tls.js',
'lib/tty.js',
'lib/url.js',
'lib/util.js',
'lib/v8.js',
'lib/vm.js',
'lib/zlib.js'
]

const labels = nodeLabels.resolveLabels(libFiles, false)

t.same(labels, libFiles.map((path) => /lib\/(_)?(\w+)\.js/.exec(path)[2]))

t.end()
})

tap.test('label: lib internals without "lib / src" limiting', (t) => {
const libFiles = [
'lib/internal/child_process.js',
'lib/internal/cluster.js',
'lib/internal/module.js',
'lib/internal/net.js',
'lib/internal/process.js',
'lib/internal/readline.js',
'lib/internal/repl.js',
'lib/internal/util.js'
]

const labels = nodeLabels.resolveLabels(libFiles, false)

t.same(labels, libFiles.map((path) => /lib\/internal\/(\w+)\.js/.exec(path)[1]))

t.end()
})
22 changes: 17 additions & 5 deletions test/node-labels.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ tap.test('no labels: when ./test/ and ./doc/ files has been changed', (t) => {
// https://github.com/nodejs/node/pull/6448
tap.test('no labels: when ./test/ and ./lib/ files has been changed', (t) => {
const labels = nodeLabels.resolveLabels([
'lib/assert.js',
'lib/punycode.js',
'test/parallel/test-assert.js'
])

Expand Down Expand Up @@ -128,22 +128,34 @@ tap.test('label: "repl" when ./lib/repl.js has been changed', (t) => {
'test/debugger/test-debugger-repl-term.js'
])

t.same(labels, ['repl'], { todo: true })
t.same(labels, ['repl'])

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()
})
Expand Down

0 comments on commit d8f0a38

Please sign in to comment.