From 378436135252b99bc120fbc0fb6c68c627cfe73c Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 12 Dec 2016 21:35:17 -0800 Subject: [PATCH 1/3] test: refactor test-fs-read-stream-inherit Refactor to take advantage of block scoping to isolate tests. Checks in exit handlers now reside with the relevant test block. Where test cases start and end is more clear. Also: Some use of `common.mustCall()` and improved wrapping/indentation. --- test/parallel/test-fs-read-stream-inherit.js | 311 ++++++++++--------- 1 file changed, 163 insertions(+), 148 deletions(-) diff --git a/test/parallel/test-fs-read-stream-inherit.js b/test/parallel/test-fs-read-stream-inherit.js index 40ec5ed883829b..3ff85ea5ef0c94 100644 --- a/test/parallel/test-fs-read-stream-inherit.js +++ b/test/parallel/test-fs-read-stream-inherit.js @@ -7,174 +7,189 @@ const fs = require('fs'); const fn = path.join(common.fixturesDir, 'elipses.txt'); const rangeFile = path.join(common.fixturesDir, 'x.txt'); -const callbacks = { open: 0, end: 0, close: 0 }; - let paused = false; -const file = fs.ReadStream(fn); +{ + const file = fs.ReadStream(fn); -file.on('open', function(fd) { - file.length = 0; - callbacks.open++; - assert.strictEqual(typeof fd, 'number'); - assert.ok(file.readable); + file.on('open', common.mustCall(function(fd) { + file.length = 0; + assert.strictEqual(typeof fd, 'number'); + assert.ok(file.readable); - // GH-535 - file.pause(); - file.resume(); - file.pause(); - file.resume(); -}); + // GH-535 + file.pause(); + file.resume(); + file.pause(); + file.resume(); + })); -file.on('data', function(data) { - assert.ok(data instanceof Buffer); - assert.ok(!paused); - file.length += data.length; + file.on('data', function(data) { + assert.ok(data instanceof Buffer); + assert.ok(!paused); + file.length += data.length; - paused = true; - file.pause(); + paused = true; + file.pause(); - setTimeout(function() { - paused = false; - file.resume(); - }, 10); -}); + setTimeout(function() { + paused = false; + file.resume(); + }, 10); + }); -file.on('end', function(chunk) { - callbacks.end++; -}); + file.on('end', common.mustCall(function() {})); -file.on('close', function() { - callbacks.close++; -}); + file.on('close', common.mustCall(function() {})); -const file3 = fs.createReadStream(fn, Object.create({encoding: 'utf8'})); -file3.length = 0; -file3.on('data', function(data) { - assert.strictEqual(typeof data, 'string'); - file3.length += data.length; + process.on('exit', function() { + assert.strictEqual(file.length, 30000); + }); +} - for (let i = 0; i < data.length; i++) { - // http://www.fileformat.info/info/unicode/char/2026/index.htm - assert.strictEqual(data[i], '\u2026'); - } -}); - -file3.on('close', function() { - callbacks.close++; -}); - -process.on('exit', function() { - assert.strictEqual(callbacks.open, 1); - assert.strictEqual(callbacks.end, 1); - assert.strictEqual(callbacks.close, 2); - assert.strictEqual(file.length, 30000); - assert.strictEqual(file3.length, 10000); - console.error('ok'); -}); - -const file4 = fs.createReadStream(rangeFile, Object.create({bufferSize: 1, - start: 1, end: 2})); -assert.strictEqual(file4.start, 1); -assert.strictEqual(file4.end, 2); -let contentRead = ''; -file4.on('data', function(data) { - contentRead += data.toString('utf-8'); -}); -file4.on('end', function(data) { - assert.strictEqual(contentRead, 'yz'); -}); - -const file5 = fs.createReadStream(rangeFile, Object.create({bufferSize: 1, - start: 1})); -assert.strictEqual(file5.start, 1); -file5.data = ''; -file5.on('data', function(data) { - file5.data += data.toString('utf-8'); -}); -file5.on('end', function() { - assert.strictEqual(file5.data, 'yz\n'); -}); +{ + const file3 = fs.createReadStream(fn, Object.create({encoding: 'utf8'})); + file3.length = 0; + file3.on('data', function(data) { + assert.strictEqual(typeof data, 'string'); + file3.length += data.length; + + for (let i = 0; i < data.length; i++) { + // http://www.fileformat.info/info/unicode/char/2026/index.htm + assert.strictEqual(data[i], '\u2026'); + } + }); + + file3.on('close', common.mustCall(function() {})); + + process.on('exit', function() { + assert.strictEqual(file3.length, 10000); + }); +} + +{ + const options = Object.create({bufferSize: 1, start: 1, end: 2}); + const file4 = fs.createReadStream(rangeFile, options); + assert.strictEqual(file4.start, 1); + assert.strictEqual(file4.end, 2); + let contentRead = ''; + file4.on('data', function(data) { + contentRead += data.toString('utf-8'); + }); + file4.on('end', function() { + assert.strictEqual(contentRead, 'yz'); + }); +} + +{ + const options = Object.create({bufferSize: 1, start: 1}); + const file5 = fs.createReadStream(rangeFile, options); + assert.strictEqual(file5.start, 1); + file5.data = ''; + file5.on('data', function(data) { + file5.data += data.toString('utf-8'); + }); + file5.on('end', function() { + assert.strictEqual(file5.data, 'yz\n'); + }); +} // https://github.com/joyent/node/issues/2320 -const file6 = fs.createReadStream(rangeFile, Object.create({bufferSize: 1.23, - start: 1})); -assert.strictEqual(file6.start, 1); -file6.data = ''; -file6.on('data', function(data) { - file6.data += data.toString('utf-8'); -}); -file6.on('end', function() { - assert.strictEqual(file6.data, 'yz\n'); -}); - -assert.throws(function() { - fs.createReadStream(rangeFile, Object.create({start: 10, end: 2})); -}, /"start" option must be <= "end" option/); - -const stream = fs.createReadStream(rangeFile, Object.create({ start: 0, - end: 0 })); -assert.strictEqual(stream.start, 0); -assert.strictEqual(stream.end, 0); -stream.data = ''; - -stream.on('data', function(chunk) { - stream.data += chunk; -}); - -stream.on('end', function() { - assert.strictEqual(stream.data, 'x'); -}); +{ + const options = Object.create({bufferSize: 1.23, start: 1}); + const file6 = fs.createReadStream(rangeFile, options); + assert.strictEqual(file6.start, 1); + file6.data = ''; + file6.on('data', function(data) { + file6.data += data.toString('utf-8'); + }); + file6.on('end', function() { + assert.strictEqual(file6.data, 'yz\n'); + }); +} -// pause and then resume immediately. -const pauseRes = fs.createReadStream(rangeFile); -pauseRes.pause(); -pauseRes.resume(); - -let file7 = fs.createReadStream(rangeFile, Object.create({autoClose: false })); -assert.strictEqual(file7.autoClose, false); -file7.on('data', function() {}); -file7.on('end', function() { - process.nextTick(function() { - assert(!file7.closed); - assert(!file7.destroyed); - file7Next(); - }); -}); - -function file7Next() { - // This will tell us if the fd is usable again or not. - file7 = fs.createReadStream(null, Object.create({fd: file7.fd, start: 0 })); - file7.data = ''; - file7.on('data', function(data) { - file7.data += data; - }); - file7.on('end', function(err) { - assert.strictEqual(file7.data, 'xyz\n'); +{ + assert.throws(function() { + fs.createReadStream(rangeFile, Object.create({start: 10, end: 2})); + }, /"start" option must be <= "end" option/); +} + +{ + const options = Object.create({start: 0, end: 0}); + const stream = fs.createReadStream(rangeFile, options); + assert.strictEqual(stream.start, 0); + assert.strictEqual(stream.end, 0); + stream.data = ''; + + stream.on('data', function(chunk) { + stream.data += chunk; + }); + + stream.on('end', function() { + assert.strictEqual(stream.data, 'x'); }); } -// Just to make sure autoClose won't close the stream because of error. -const file8 = fs.createReadStream(null, Object.create({fd: 13337, - autoClose: false })); -file8.on('data', function() {}); -file8.on('error', common.mustCall(function() {})); +// pause and then resume immediately. +{ + const pauseRes = fs.createReadStream(rangeFile); + pauseRes.pause(); + pauseRes.resume(); +} -// Make sure stream is destroyed when file does not exist. -const file9 = fs.createReadStream('/path/to/file/that/does/not/exist'); -file9.on('data', function() {}); -file9.on('error', common.mustCall(function() {})); +{ + let file7 = + fs.createReadStream(rangeFile, Object.create({autoClose: false })); + assert.strictEqual(file7.autoClose, false); + file7.on('data', function() {}); + file7.on('end', function() { + process.nextTick(function() { + assert(!file7.closed); + assert(!file7.destroyed); + file7Next(); + }); + }); -process.on('exit', function() { - assert(file7.closed); - assert(file7.destroyed); + function file7Next() { + // This will tell us if the fd is usable again or not. + file7 = fs.createReadStream(null, Object.create({fd: file7.fd, start: 0 })); + file7.data = ''; + file7.on('data', function(data) { + file7.data += data; + }); + file7.on('end', function() { + assert.strictEqual(file7.data, 'xyz\n'); + }); + } + process.on('exit', function() { + assert(file7.closed); + assert(file7.destroyed); + }); +} - assert(!file8.closed); - assert(!file8.destroyed); - assert(file8.fd); +// Just to make sure autoClose won't close the stream because of error. +{ + const options = Object.create({fd: 13337, autoClose: false}); + const file8 = fs.createReadStream(null, options); + file8.on('data', function() {}); + file8.on('error', common.mustCall(function() {})); + process.on('exit', function() { + assert(!file8.closed); + assert(!file8.destroyed); + assert(file8.fd); + }); +} - assert(!file9.closed); - assert(file9.destroyed); -}); +// Make sure stream is destroyed when file does not exist. +{ + const file9 = fs.createReadStream('/path/to/file/that/does/not/exist'); + file9.on('data', function() {}); + file9.on('error', common.mustCall(function() {})); + + process.on('exit', function() { + assert(!file9.closed); + assert(file9.destroyed); + }); +} From cbceb8dde9eed7a0589145429a833f02080c9109 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 12 Dec 2016 22:27:24 -0800 Subject: [PATCH 2/3] squash: moar common.mustCall() --- test/parallel/test-fs-read-stream-inherit.js | 28 ++++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/test/parallel/test-fs-read-stream-inherit.js b/test/parallel/test-fs-read-stream-inherit.js index 3ff85ea5ef0c94..c8c4049dcb3569 100644 --- a/test/parallel/test-fs-read-stream-inherit.js +++ b/test/parallel/test-fs-read-stream-inherit.js @@ -78,9 +78,9 @@ let paused = false; file4.on('data', function(data) { contentRead += data.toString('utf-8'); }); - file4.on('end', function() { + file4.on('end', common.mustCall(function() { assert.strictEqual(contentRead, 'yz'); - }); + })); } { @@ -91,9 +91,9 @@ let paused = false; file5.on('data', function(data) { file5.data += data.toString('utf-8'); }); - file5.on('end', function() { + file5.on('end', common.mustCall(function() { assert.strictEqual(file5.data, 'yz\n'); - }); + })); } // https://github.com/joyent/node/issues/2320 @@ -105,9 +105,9 @@ let paused = false; file6.on('data', function(data) { file6.data += data.toString('utf-8'); }); - file6.on('end', function() { + file6.on('end', common.mustCall(function() { assert.strictEqual(file6.data, 'yz\n'); - }); + })); } { @@ -127,9 +127,9 @@ let paused = false; stream.data += chunk; }); - stream.on('end', function() { + stream.on('end', common.mustCall(function() { assert.strictEqual(stream.data, 'x'); - }); + })); } // pause and then resume immediately. @@ -144,13 +144,13 @@ let paused = false; fs.createReadStream(rangeFile, Object.create({autoClose: false })); assert.strictEqual(file7.autoClose, false); file7.on('data', function() {}); - file7.on('end', function() { - process.nextTick(function() { + file7.on('end', common.mustCall(function() { + process.nextTick(common.mustCall(function() { assert(!file7.closed); assert(!file7.destroyed); file7Next(); - }); - }); + })); + })); function file7Next() { // This will tell us if the fd is usable again or not. @@ -159,9 +159,9 @@ let paused = false; file7.on('data', function(data) { file7.data += data; }); - file7.on('end', function() { + file7.on('end', common.mustCall(function() { assert.strictEqual(file7.data, 'xyz\n'); - }); + })); } process.on('exit', function() { assert(file7.closed); From c5cf4eddaae236f476dc7739aa8038629288b02e Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 14 Dec 2016 15:19:03 -0800 Subject: [PATCH 3/3] squash: nits from targos --- test/parallel/test-fs-read-stream-inherit.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/test/parallel/test-fs-read-stream-inherit.js b/test/parallel/test-fs-read-stream-inherit.js index c8c4049dcb3569..d71dc3d4381272 100644 --- a/test/parallel/test-fs-read-stream-inherit.js +++ b/test/parallel/test-fs-read-stream-inherit.js @@ -42,11 +42,9 @@ let paused = false; file.on('end', common.mustCall(function() {})); - file.on('close', common.mustCall(function() {})); - - process.on('exit', function() { + file.on('close', common.mustCall(function() { assert.strictEqual(file.length, 30000); - }); + })); } { @@ -62,11 +60,9 @@ let paused = false; } }); - file3.on('close', common.mustCall(function() {})); - - process.on('exit', function() { + file3.on('close', common.mustCall(function() { assert.strictEqual(file3.length, 10000); - }); + })); } {