From 3eea668c7a5b5b89ed052c8bdeda4f5b6277f311 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 29 Oct 2016 23:24:10 -0700 Subject: [PATCH 1/9] test: increase test coverage for lib/zlib.js Add tests for constructor behavior and parameter validation. Remove condition check that cannot be triggered (nothing is greater than `Infinity`). PR-URL: https://github.com/nodejs/node/pull/9366 Reviewed-By: Colin Ihrig Reviewed-By: Minwoo Jung Reviewed-By: James M Snell --- lib/zlib.js | 3 +- .../test-zlib-deflate-constructors.js | 105 ++++++++++++++++++ 2 files changed, 106 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-zlib-deflate-constructors.js diff --git a/lib/zlib.js b/lib/zlib.js index 42dd561623cb75..bdce0c8d7f2eec 100644 --- a/lib/zlib.js +++ b/lib/zlib.js @@ -300,8 +300,7 @@ function Zlib(opts, mode) { opts.finishFlush : constants.Z_FINISH; if (opts.chunkSize) { - if (opts.chunkSize < constants.Z_MIN_CHUNK || - opts.chunkSize > constants.Z_MAX_CHUNK) { + if (opts.chunkSize < constants.Z_MIN_CHUNK) { throw new Error('Invalid chunk size: ' + opts.chunkSize); } } diff --git a/test/parallel/test-zlib-deflate-constructors.js b/test/parallel/test-zlib-deflate-constructors.js new file mode 100644 index 00000000000000..2f23dc595ec0b0 --- /dev/null +++ b/test/parallel/test-zlib-deflate-constructors.js @@ -0,0 +1,105 @@ +'use strict'; + +require('../common'); + +const zlib = require('zlib'); +const assert = require('assert'); + +// Work with and without `new` keyword +assert.ok(zlib.Deflate() instanceof zlib.Deflate); +assert.ok(new zlib.Deflate() instanceof zlib.Deflate); + +assert.ok(zlib.DeflateRaw() instanceof zlib.DeflateRaw); +assert.ok(new zlib.DeflateRaw() instanceof zlib.DeflateRaw); + +// Throws if `opts.chunkSize` is invalid +assert.throws( + () => { new zlib.Deflate({chunkSize: -Infinity}); }, + /^Error: Invalid chunk size: -Infinity$/ +); + +// Confirm that maximum chunk size cannot be exceeded because it is `Infinity`. +assert.strictEqual(zlib.constants.Z_MAX_CHUNK, Infinity); + +// Throws if `opts.windowBits` is invalid +assert.throws( + () => { new zlib.Deflate({windowBits: -Infinity}); }, + /^Error: Invalid windowBits: -Infinity$/ +); + +assert.throws( + () => { new zlib.Deflate({windowBits: Infinity}); }, + /^Error: Invalid windowBits: Infinity$/ +); + +// Throws if `opts.level` is invalid +assert.throws( + () => { new zlib.Deflate({level: -Infinity}); }, + /^Error: Invalid compression level: -Infinity$/ +); + +assert.throws( + () => { new zlib.Deflate({level: Infinity}); }, + /^Error: Invalid compression level: Infinity$/ +); + +// Throws a RangeError if `level` invalid in `Deflate.prototype.params()` +assert.throws( + () => { new zlib.Deflate().params(-Infinity); }, + /^RangeError: Invalid compression level: -Infinity$/ +); + +assert.throws( + () => { new zlib.Deflate().params(Infinity); }, + /^RangeError: Invalid compression level: Infinity$/ +); + +// Throws if `opts.memLevel` is invalid +assert.throws( + () => { new zlib.Deflate({memLevel: -Infinity}); }, + /^Error: Invalid memLevel: -Infinity$/ +); + +assert.throws( + () => { new zlib.Deflate({memLevel: Infinity}); }, + /^Error: Invalid memLevel: Infinity$/ +); + +// Does not throw if opts.strategy is valid +assert.doesNotThrow( + () => { new zlib.Deflate({strategy: zlib.constants.Z_FILTERED}); } +); + +assert.doesNotThrow( + () => { new zlib.Deflate({strategy: zlib.constants.Z_HUFFMAN_ONLY}); } +); + +assert.doesNotThrow( + () => { new zlib.Deflate({strategy: zlib.constants.Z_RLE}); } +); + +assert.doesNotThrow( + () => { new zlib.Deflate({strategy: zlib.constants.Z_FIXED}); } +); + +assert.doesNotThrow( + () => { new zlib.Deflate({ strategy: zlib.constants.Z_DEFAULT_STRATEGY}); } +); + +// Throws if opts.strategy is invalid +assert.throws( + () => { new zlib.Deflate({strategy: 'this is a bogus strategy'}); }, + /^Error: Invalid strategy: this is a bogus strategy$/ +); + +// Throws TypeError if `strategy` is invalid in `Deflate.prototype.params()` +assert.throws( + () => { new zlib.Deflate().params(0, 'I am an invalid strategy'); }, + /^TypeError: Invalid strategy: I am an invalid strategy$/ +); + +// Throws if opts.dictionary is not a Buffer +assert.throws( + () => { new zlib.Deflate({dictionary: 'not a buffer'}); }, + /^Error: Invalid dictionary: it should be a Buffer instance$/ +); From 45a716c9687849b7ade2ce01f2421c8f791f907c Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 29 Oct 2016 23:24:33 +0200 Subject: [PATCH 2/9] test: remove timers from streams test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test-stream2-readable-empty-buffer-no-eof fails on resource-constrained machines due to its use of timers. Removing timers makes it more reliable and doesn’t affect the validity of the test, as it only uses relative timing relations. Failures were noticed on freebsd10-64 in CI. I am able to replicate the failure with `tools/test.py --repeat=100 -j 100`. When run alone, it passes reliably. Refs: https://github.com/nodejs/node/pull/9359 PR-URL: hkttps://github.com/nodejs/node/pull/9360 Reviewed-By: James M Snell Reviewed-By: Rich Trott Reviewed-By: Ilkka Myller Reviewed-By: Colin Ihrig --- ...st-stream2-readable-empty-buffer-no-eof.js | 37 +++++++++---------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/test/parallel/test-stream2-readable-empty-buffer-no-eof.js b/test/parallel/test-stream2-readable-empty-buffer-no-eof.js index 61d3096ef18620..ccbf087df00bf5 100644 --- a/test/parallel/test-stream2-readable-empty-buffer-no-eof.js +++ b/test/parallel/test-stream2-readable-empty-buffer-no-eof.js @@ -1,5 +1,5 @@ 'use strict'; -const common = require('../common'); +require('../common'); const assert = require('assert'); const Readable = require('stream').Readable; @@ -16,36 +16,35 @@ function test1() { // // note that this is very unusual. it only works for crypto streams // because the other side of the stream will call read(0) to cycle - // data through openssl. that's why we set the timeouts to call + // data through openssl. that's why setImmediate() is used to call // r.read(0) again later, otherwise there is no more work being done // and the process just exits. const buf = Buffer.alloc(5, 'x'); let reads = 5; - const timeout = common.platformTimeout(50); r._read = function(n) { switch (reads--) { - case 0: - return r.push(null); // EOF - case 1: - return r.push(buf); - case 2: - setTimeout(r.read.bind(r, 0), timeout); - return r.push(Buffer.alloc(0)); // Not-EOF! - case 3: - setTimeout(r.read.bind(r, 0), timeout); - return process.nextTick(function() { - return r.push(Buffer.alloc(0)); + case 5: + return setImmediate(function() { + return r.push(buf); }); case 4: - setTimeout(r.read.bind(r, 0), timeout); - return setTimeout(function() { + setImmediate(function() { return r.push(Buffer.alloc(0)); }); - case 5: - return setTimeout(function() { - return r.push(buf); + return setImmediate(r.read.bind(r, 0)); + case 3: + setImmediate(r.read.bind(r, 0)); + return process.nextTick(function() { + return r.push(Buffer.alloc(0)); }); + case 2: + setImmediate(r.read.bind(r, 0)); + return r.push(Buffer.alloc(0)); // Not-EOF! + case 1: + return r.push(buf); + case 0: + return r.push(null); // EOF default: throw new Error('unreachable'); } From 0fb21df6e692bef9f55b9bfa876f3c59dc590332 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 25 Oct 2016 22:13:24 +0200 Subject: [PATCH 3/9] lib: make `String(global) === '[object global]'` This inadvertently changed to `[object Object]` with the V8 upgrade in 8a24728...96933df. Use `Symbol.toStringTag` to undo this particular change. Fixes: https://github.com/nodejs/node/issues/9274 PR-URL: https://github.com/nodejs/node/pull/9279 Reviewed-By: Prince John Wesley Reviewed-By: Ben Noordhuis Reviewed-By: James M Snell Reviewed-By: Colin Ihrig Reviewed-By: Franziska Hinkelmann --- lib/internal/bootstrap_node.js | 6 ++++++ test/parallel/test-global.js | 2 ++ 2 files changed, 8 insertions(+) diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js index 2b7e010214d258..4e1e2aa9018b5a 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -190,6 +190,12 @@ } function setupGlobalVariables() { + Object.defineProperty(global, Symbol.toStringTag, { + value: 'global', + writable: false, + enumerable: false, + configurable: true + }); global.process = process; const util = NativeModule.require('util'); diff --git a/test/parallel/test-global.js b/test/parallel/test-global.js index 1bf1bd7cd69303..270faf1b7768a6 100644 --- a/test/parallel/test-global.js +++ b/test/parallel/test-global.js @@ -21,3 +21,5 @@ const fooBar = module.fooBar; assert.strictEqual('foo', fooBar.foo, 'x -> global.x in sub level not working'); assert.strictEqual('bar', fooBar.bar, 'global.x -> x in sub level not working'); + +assert.strictEqual(Object.prototype.toString.call(global), '[object global]'); From 49d1c366d85f4b7e321b1d71eabaae59c20b83c5 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Fri, 28 Oct 2016 14:02:24 -0400 Subject: [PATCH 4/9] child_process: remove extra newline in errors checkExecSyncError() creates error objects for execSync() and execFileSync(). If the child process created stderr output, then it is attached to the end of the error message. However, stderr can be an empty Buffer object, which always passes the truthy check, leading to an extra newline in the error message. This commit adds a length check, which will work with both strings and Buffers. PR-URL: https://github.com/nodejs/node/pull/9343 Reviewed-By: Wyatt Preul Reviewed-By: James M Snell --- lib/child_process.js | 2 +- test/sequential/test-child-process-execsync.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/child_process.js b/lib/child_process.js index afdc5ae551a95e..a108cba0201cdb 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -483,7 +483,7 @@ function checkExecSyncError(ret) { if (!err) { var msg = 'Command failed: '; msg += ret.cmd || ret.args.join(' '); - if (ret.stderr) + if (ret.stderr && ret.stderr.length > 0) msg += '\n' + ret.stderr.toString(); err = new Error(msg); } diff --git a/test/sequential/test-child-process-execsync.js b/test/sequential/test-child-process-execsync.js index 137ddbda425f2c..e40651dacff0a1 100644 --- a/test/sequential/test-child-process-execsync.js +++ b/test/sequential/test-child-process-execsync.js @@ -98,7 +98,7 @@ assert.strictEqual(ret, msg + '\n', const msg = `Command failed: ${process.execPath} ${args.join(' ')}`; assert(err instanceof Error); - assert.strictEqual(err.message.trim(), msg); + assert.strictEqual(err.message, msg); assert.strictEqual(err.status, 1); return true; }); From 1f08d354c5bd2ecca5f106c90e93f0fee3c352d7 Mon Sep 17 00:00:00 2001 From: Josh Gavant Date: Thu, 27 Oct 2016 14:33:08 -0700 Subject: [PATCH 5/9] doc: update Diagnostics WG info Updates info on Diagnostics WG (https://github.com/nodejs/diagnostics). PR-URL: https://github.com/nodejs/node/pull/9329 Reviewed-By: Michael Dawson Reviewed-By: James M Snell Reviewed-By: Colin Ihrig --- WORKING_GROUPS.md | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/WORKING_GROUPS.md b/WORKING_GROUPS.md index dd2319ce8f1adb..ab24942aecffcf 100644 --- a/WORKING_GROUPS.md +++ b/WORKING_GROUPS.md @@ -21,7 +21,7 @@ back in to the CTC. * [Website](#website) * [Streams](#streams) * [Build](#build) -* [Tracing](#tracing) +* [Diagnostics](#diagnostics) * [i18n](#i18n) * [Evangelism](#evangelism) * [Roadmap](#roadmap) @@ -81,17 +81,22 @@ Its responsibilities are: * Creates and manages build-containers. -### [Tracing](https://github.com/nodejs/tracing-wg) +### [Diagnostics](https://github.com/nodejs/diagnostics) -The tracing working group's purpose is to increase the -transparency of software written in Node.js. +The diagnostics working group's purpose is to surface a set of comprehensive, +documented, and extensible diagnostic interfaces for use by +Node.js tools and JavaScript VMs. Its responsibilities are: -* Collaboration with V8 to integrate with `trace_event`. -* Maintenance and iteration on AsyncWrap. -* Maintenance and improvements to system tracing support (DTrace, LTTng, etc.) -* Documentation of tracing and debugging techniques. -* Fostering a tracing and debugging ecosystem. + +* Collaborate with V8 to integrate `v8_inspector` into Node.js. +* Collaborate with V8 to integrate `trace_event` into Node.js. +* Collaborate with Core to refine `async_wrap` and `async_hooks`. +* Maintain and improve OS trace system integration (e.g. ETW, LTTNG, dtrace). +* Document diagnostic capabilities and APIs in Node.js and its components. +* Explore opportunities and gaps, discuss feature requests, and address + conflicts in Node.js diagnostics. +* Foster an ecosystem of diagnostics tools for Node.js. ### i18n From 3c1e5b366f92a51824c5ae0b6f20cd290cf54a0e Mon Sep 17 00:00:00 2001 From: Rod Vagg Date: Thu, 3 Nov 2016 07:41:40 +1100 Subject: [PATCH 6/9] doc: add Sakthipriyan to the CTC PR-URL: https://github.com/nodejs/node/pull/9427 Reviewed-By: Myles Borins Reviewed-By: Evan Lucas Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Rich Trott Reviewed-By: Jeremiah Senkpiel Reviewed-By: Johan Bergstrom --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index d15916e2ff09c5..caf0d24d48cf08 100644 --- a/README.md +++ b/README.md @@ -186,6 +186,8 @@ more information about the governance of the Node.js project, see **Shigeki Ohtsu** <ohtsu@iij.ad.jp> * [TheAlphaNerd](https://github.com/TheAlphaNerd) - **Myles Borins** <myles.borins@gmail.com> +* [thefourtheye](https://github.com/thefourtheye) - +**Sakthipriyan Vairamani** <thechargingvolcano@gmail.com> * [trevnorris](https://github.com/trevnorris) - **Trevor Norris** <trev.norris@gmail.com> * [Trott](https://github.com/Trott) - @@ -319,8 +321,6 @@ more information about the governance of the Node.js project, see **Michaël Zasso** <targos@protonmail.com> * [tellnes](https://github.com/tellnes) - **Christian Tellnes** <christian@tellnes.no> -* [thefourtheye](https://github.com/thefourtheye) - -**Sakthipriyan Vairamani** <thechargingvolcano@gmail.com> * [thekemkid](https://github.com/thekemkid) - **Glen Keane** <glenkeane.94@gmail.com> * [thlorenz](https://github.com/thlorenz) - From 5cbb7a887a7ddb737252ecf60fbebc2005af090c Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sun, 30 Oct 2016 20:12:25 -0700 Subject: [PATCH 7/9] repl: refactor lib/repl.js * remove unnecessary backslash (`\`) escaping in regular expressions * favor `===` over `==` * multiline arrays indentation consistent with other indentation PR-URL: https://github.com/nodejs/node/pull/9374 Reviewed-By: Rod Vagg Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Roman Reiss --- lib/repl.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/repl.js b/lib/repl.js index d00757b5125571..7a735371ffd66a 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -40,12 +40,12 @@ const parentModule = module; const replMap = new WeakMap(); const GLOBAL_OBJECT_PROPERTIES = ['NaN', 'Infinity', 'undefined', - 'eval', 'parseInt', 'parseFloat', 'isNaN', 'isFinite', 'decodeURI', - 'decodeURIComponent', 'encodeURI', 'encodeURIComponent', - 'Object', 'Function', 'Array', 'String', 'Boolean', 'Number', - 'Date', 'RegExp', 'Error', 'EvalError', 'RangeError', - 'ReferenceError', 'SyntaxError', 'TypeError', 'URIError', - 'Math', 'JSON']; + 'eval', 'parseInt', 'parseFloat', 'isNaN', 'isFinite', 'decodeURI', + 'decodeURIComponent', 'encodeURI', 'encodeURIComponent', + 'Object', 'Function', 'Array', 'String', 'Boolean', 'Number', + 'Date', 'RegExp', 'Error', 'EvalError', 'RangeError', + 'ReferenceError', 'SyntaxError', 'TypeError', 'URIError', + 'Math', 'JSON']; const GLOBAL_OBJECT_PROPERTY_MAP = {}; GLOBAL_OBJECT_PROPERTIES.forEach((p) => GLOBAL_OBJECT_PROPERTY_MAP[p] = p); @@ -783,7 +783,7 @@ ArrayStream.prototype.writable = true; ArrayStream.prototype.resume = function() {}; ArrayStream.prototype.write = function() {}; -const requireRE = /\brequire\s*\(['"](([\w\.\/-]+\/)?([\w\.\/-]*))/; +const requireRE = /\brequire\s*\(['"](([\w./-]+\/)?([\w./-]*))/; const simpleExpressionRE = /(([a-zA-Z_$](?:\w|\$)*)\.)*([a-zA-Z_$](?:\w|\$)*)\.?$/; @@ -1036,7 +1036,7 @@ function complete(line, callback) { var newCompletionGroups = []; for (i = 0; i < completionGroups.length; i++) { group = completionGroups[i].filter(function(elem) { - return elem.indexOf(filter) == 0; + return elem.indexOf(filter) === 0; }); if (group.length) { newCompletionGroups.push(group); @@ -1341,8 +1341,8 @@ function regexpEscape(s) { // TODO(princejwesley): Remove it prior to v8.0.0 release // Reference: https://github.com/nodejs/node/pull/7829 REPLServer.prototype.convertToContext = util.deprecate(function(cmd) { - const scopeVar = /^\s*var\s*([_\w\$]+)(.*)$/m; - const scopeFunc = /^\s*function\s*([_\w\$]+)/; + const scopeVar = /^\s*var\s*([\w$]+)(.*)$/m; + const scopeFunc = /^\s*function\s*([\w$]+)/; var matches; // Replaces: var foo = "bar"; with: self.context.foo = bar; From 7537718460c7b964ffbbc0910b12eaff9cd8b7a8 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Tue, 1 Nov 2016 14:41:16 -0700 Subject: [PATCH 8/9] tools: enforce function name matching in linter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ESLint has a `func-name-matching` rule that requires that function names match the variable or property to which they are being assigned. The code base currently has 100% compliance with this rule. Enable the rule to keep it that way. PR-URL: https://github.com/nodejs/node/pull/9408 Reviewed-By: Roman Reiss Reviewed-By: Colin Ihrig Reviewed-By: Michaël Zasso Reviewed-By: Gibson Fahnestock Reviewed-By: Teddy Katz Reviewed-By: Prince John Wesley --- .eslintrc | 1 + 1 file changed, 1 insertion(+) diff --git a/.eslintrc b/.eslintrc index fdfdaf0833045d..337ff4f35de17c 100644 --- a/.eslintrc +++ b/.eslintrc @@ -82,6 +82,7 @@ rules: computed-property-spacing: 2 eol-last: 2 func-call-spacing: 2 + func-name-matching: 2 indent: [2, 2, {SwitchCase: 1, MemberExpression: 1}] key-spacing: [2, {mode: minimum}] keyword-spacing: 2 From 45af60e2e7e48ce460ae8f140a1198dee50b2e4a Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Fri, 4 Nov 2016 13:05:02 -0400 Subject: [PATCH 9/9] test commit 3 --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index caf0d24d48cf08..231aec5e6b8565 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,7 @@ # Node.js +// HI THIS SHOULD NOT CAUSE CONFLICTS + [![Gitter](https://badges.gitter.im/Join Chat.svg)](https://gitter.im/nodejs/node?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge) [![CII Best Practices](https://bestpractices.coreinfrastructure.org/projects/29/badge)](https://bestpractices.coreinfrastructure.org/projects/29) Node.js is a JavaScript runtime built on Chrome's V8 JavaScript engine. Node.js