From 8f316859a51871ae2dea569f2aa60dd342a55e4b Mon Sep 17 00:00:00 2001 From: Evan Lucas Date: Thu, 28 Apr 2016 14:58:42 -0500 Subject: [PATCH 1/6] dir-reader: account for entries being changed after _read If this.entries is changed after _read() has been called, we will be out of sync and try to access an invalid index of this.entries. When the entry cannot be found, we emit end and close, which can drop files from reading. --- lib/dir-reader.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/dir-reader.js b/lib/dir-reader.js index 820cdc8..b220658 100644 --- a/lib/dir-reader.js +++ b/lib/dir-reader.js @@ -24,6 +24,7 @@ function DirReader (props) { } self.entries = null + self._entries = [] self._index = -1 self._paused = false self._length = -1 @@ -47,6 +48,7 @@ DirReader.prototype._getEntries = function () { if (er) return self.error(er) self.entries = entries + self._entries = entries.slice() self.emit('entries', entries) if (self._paused) self.once('resume', processEntries) @@ -74,7 +76,7 @@ DirReader.prototype._read = function () { } self._index++ - if (self._index >= self.entries.length) { + if (self._index >= self._entries.length) { if (!self._ended) { self._ended = true self.emit('end') @@ -83,12 +85,14 @@ DirReader.prototype._read = function () { return } - // ok, handle this one, then. - // save creating a proxy, by stat'ing the thing now. - var p = path.resolve(self._path, self.entries[self._index]) + var nextEntry = self._entries[self._index] + if (!nextEntry) return this._read() + + // ok, handle this one, then. + var p = path.resolve(self._path, nextEntry) assert(p !== self._path) - assert(self.entries[self._index]) + assert(nextEntry) // set this to prevent trying to _read() again in the stat time. self._currentEntry = p From 9dab82beb2a36e1768aafee000c3134053f4bdb5 Mon Sep 17 00:00:00 2001 From: Forrest L Norvell Date: Thu, 28 Apr 2016 20:31:47 -0700 Subject: [PATCH 2/6] deps: upgrade to newest mkdirp & rimraf --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index cd0cd44..c06da16 100644 --- a/package.json +++ b/package.json @@ -14,8 +14,8 @@ "dependencies": { "graceful-fs": "^4.1.2", "inherits": "~2.0.0", - "mkdirp": ">=0.5 0", - "rimraf": "2" + "mkdirp": "^0.5.1", + "rimraf": "^2.5.2" }, "devDependencies": { "standard": "^4.0.0", From d44b94328c7bcedd3283a2fe903b855083ff23b3 Mon Sep 17 00:00:00 2001 From: Forrest L Norvell Date: Thu, 28 Apr 2016 20:36:01 -0700 Subject: [PATCH 3/6] deps: standard@6.0.8 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index c06da16..40dae55 100644 --- a/package.json +++ b/package.json @@ -18,7 +18,7 @@ "rimraf": "^2.5.2" }, "devDependencies": { - "standard": "^4.0.0", + "standard": "^6.0.8", "tap": "^1.2.0" }, "scripts": { From 54a6ba79bc7b44596a054defe4bc9a488716370e Mon Sep 17 00:00:00 2001 From: Forrest L Norvell Date: Thu, 28 Apr 2016 20:37:04 -0700 Subject: [PATCH 4/6] deps: tap@5.7.1 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 40dae55..b9ae7ea 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,7 @@ }, "devDependencies": { "standard": "^6.0.8", - "tap": "^1.2.0" + "tap": "^5.7.1" }, "scripts": { "test": "standard && tap examples/*.js" From b95f1f7fc5669ab553e0b2393530daf5643572fe Mon Sep 17 00:00:00 2001 From: Forrest L Norvell Date: Thu, 28 Apr 2016 23:04:05 -0700 Subject: [PATCH 5/6] test: unit tests for Abstract, with documentation --- .gitignore | 2 + README.md | 88 ++++++++++++++++++++++----- lib/abstract.js | 9 ++- package.json | 4 +- test/abstract.js | 155 +++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 240 insertions(+), 18 deletions(-) create mode 100644 test/abstract.js diff --git a/.gitignore b/.gitignore index 494272a..227a55d 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,5 @@ node_modules/ examples/deep-copy/ examples/path/ examples/filter-copy/ +.nyc_output/ +coverage/ diff --git a/README.md b/README.md index 9d8cb77..197d936 100644 --- a/README.md +++ b/README.md @@ -21,11 +21,12 @@ same as the intended size, if the size is set. ```javascript fstream - .Writer({ path: "path/to/file" - , mode: 0755 - , size: 6 - }) - .write("hello\n") + .Writer({ + path: 'path/to/file', + mode: parseInt('0755', 8), + size: 6 + }) + .write('hello\n') .end() ``` @@ -35,12 +36,13 @@ been written when it's done. ```javascript fstream - .Writer({ path: "path/to/file" - , mode: 0755 - , size: 6 - , flags: "a" - }) - .write("hello\n") + .Writer({ + path: 'path/to/file', + mode: parseInt('0755', 8), + size: 6, + flags: 'a' + }) + .write('hello\n') .end() ``` @@ -48,11 +50,12 @@ You can pass flags in, if you want to append to a file. ```javascript fstream - .Writer({ path: "path/to/symlink" - , linkpath: "./file" - , SymbolicLink: true - , mode: "0755" // octal strings supported - }) + .Writer({ + path: 'path/to/symlink', + linkpath: './file', + SymbolicLink: true, + mode: '0755' // octal strings supported + }) .end() ``` @@ -74,3 +77,56 @@ This will do like `cp -Rp path/to/dir path/to/other/dir`. If the other dir exists and isn't a directory, then it'll emit an error. It'll also set the uid, gid, mode, etc. to be identical. In this way, it's more like `rsync -a` than simply a copy. + +# API + +## Abstract (extends `Stream`) + +A base class that extends [`Stream`](https://nodejs.org/api/stream.html) with +useful utility methods. `fstream` streams are based on [streams1 +semantics](https://gist.github.com/caike/ebccc95bd46f5fa1404d#file-streams-1-js). + +### events + +- `abort`: Stop further processing on the stream. +- `ready`: The stream is ready for reading; handlers passed to `.on()` will + still be called if the stream is ready even if they're added after `ready` is + emitted. +- `info`: Quasi-logging event emitted for diagnostic information. +- `warn`: Quasi-logging event emitted on non-fatal errors. +- `error`: Quasi-logging event emitted on fatal errors. + +### properties + +- `ready`: Whether the current file stream is ready to start processing. _Default: `false`_ +- `path`: Path to the filesystem object this node is bound to. +- `linkpath`: Target path to which a link points. +- `type`: What type of filesystem entity this file stream node points to. + +### abstract.abort() + +Stop any further processing on the file stream by setting `this._aborted`; for +use by subclasses. + +### abstract.destroy() + +Abstract base method; overrides `Stream`'s `destroy` as a no-op. + +### abstract.info(msg, code) + +Quasi-logging method. + +Emits an `info` event with `msg` and `code` attached. + +### abstract.warn(msg, code) + +Quasi-logging method. + +Emits a `warn` event if it has any listeners; otherwise prints out an error +object decorated with `msg` and `code` to stderr. + +### abstract.error(msg, code, throw) + +If `throw` is true, throw an Error decorated with the message or code. +Otherwise, emit `error` with the decorated Error. `msg` can also be an Error +object itself; it will be wrapped in a new Error before being annotated. diff --git a/lib/abstract.js b/lib/abstract.js index 97c120e..8023768 100644 --- a/lib/abstract.js +++ b/lib/abstract.js @@ -7,6 +7,13 @@ var inherits = require('inherits') function Abstract () { Stream.call(this) + this.ready = false + this.path = null + this.linkpath = null + this.type = null + this._aborted = false + this._path = null + this._paused = false } inherits(Abstract, Stream) @@ -30,7 +37,7 @@ Abstract.prototype.destroy = function () {} Abstract.prototype.warn = function (msg, code) { var self = this var er = decorate(msg, code, self) - if (!self.listeners('warn')) { + if (!self.listeners('warn').length) { console.error('%s %s\n' + 'path = %s\n' + 'syscall = %s\n' + diff --git a/package.json b/package.json index b9ae7ea..1b57c38 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,9 @@ "tap": "^5.7.1" }, "scripts": { - "test": "standard && tap examples/*.js" + "test": "standard && tap --coverage test/*.js", + "test-legacy": "tap examples/*.js", + "coverage-report": "tap --coverage --coverage-report=html examples/*.js test/*.js" }, "license": "ISC" } diff --git a/test/abstract.js b/test/abstract.js new file mode 100644 index 0000000..e962b60 --- /dev/null +++ b/test/abstract.js @@ -0,0 +1,155 @@ +var format = require('util').format + +var test = require('tap').test + +var Abstract = require('../').Abstract + +test('basic Abstract contract', function (t) { + t.doesNotThrow(function () { + t.ok(new Abstract()) + }) + var fstream = new Abstract() + t.is(typeof fstream.on, 'function') + + // extra ways to end streams + t.is(typeof fstream.abort, 'function') + t.is(typeof fstream.destroy, 'function') + + // loggingish functions + t.is(typeof fstream.warn, 'function') + t.is(typeof fstream.info, 'function') + t.is(typeof fstream.error, 'function') + + t.end() +}) + +test('calls "ready" callbacks even after event emitted', function (t) { + var fstream = new Abstract() + fstream.ready = true + fstream.on('ready', function () { + t.is(this._aborted, false, 'this is bound correctly') + // called asap even though ready isn't emitted + t.end() + }) +}) + +test('aborting abstractly', function (t) { + var fstream = new Abstract() + // gross, but no other way to observe this state for the base class + t.is(fstream._aborted, false) + fstream.on('abort', function () { + // see above + t.is(fstream._aborted, true) + t.end() + }) + + fstream.abort() +}) + +test('destroying abstractly', function (t) { + var fstream = new Abstract() + t.doesNotThrow(function () { fstream.destroy() }, 'do nothing') + t.end() +}) + +test('informing abstractly', function (t) { + var fstream = new Abstract() + t.doesNotThrow(function () { fstream.info('hi', 'EYO') }) + fstream.on('info', function (message, code) { + t.is(message, 'yup') + t.is(code, 'EHOWDY') + t.end() + }) + + fstream.info('yup', 'EHOWDY') +}) + +test('warning abstractly', function (t) { + t.test('emits with a listener', function (t) { + var fstream = new Abstract() + fstream.path = '/dev/null' + fstream.on('warn', function (err) { + t.is(err.message, 'hi') + t.is(err.code, 'EFRIENDLY') + t.is(err.fstream_class, 'Abstract') + t.is(err.fstream_path, '/dev/null') + }) + + fstream.warn('hi', 'EFRIENDLY') + t.end() + }) + + t.test('prints without a listener', function (t) { + var fstream = new Abstract() + fstream.path = '/dev/null' + var _error = console.error + console.error = function () { + console.error = _error + var formatted = format.apply(console, [].slice.call(arguments)) + t.matches(formatted, /^EUNFRIENDLY Error: ono/) + t.matches(formatted, /fstream_class = Abstract/) + t.matches(formatted, /path = \/dev\/null/) + t.end() + } + + fstream.warn('ono', 'EUNFRIENDLY') + }) + + t.test('prints without a listener and defaults to code of UNKNOWN', function (t) { + var fstream = new Abstract() + fstream.path = '/dev/null' + var _error = console.error + console.error = function () { + console.error = _error + var formatted = format.apply(console, [].slice.call(arguments)) + t.matches(formatted, /^UNKNOWN Error: wow mom/) + t.matches(formatted, /fstream_class = Abstract/) + t.matches(formatted, /path = \/dev\/null/) + t.end() + } + + fstream.warn('wow mom') + }) + + t.end() +}) + +test('erroring abstractly', function (t) { + t.test('emits by default if handler set', function (t) { + var fstream = new Abstract() + t.throws( + function () { fstream.error('whoops', 'EYIKES') }, + { message: 'whoops', code: 'EYIKES' }, + 'streams throw if no handler is set' + ) + + fstream.linkpath = '/road/to/nowhere' + fstream.on('error', function (err) { + t.is(err.message, 'candygram!') + t.is(err.code, 'ELANDSHARK') + t.is(err.fstream_linkpath, '/road/to/nowhere') + t.end() + }) + + fstream.error(new Error('candygram!'), 'ELANDSHARK') + }) + + t.test('throws when told to do so', function (t) { + var fstream = new Abstract() + + fstream.linkpath = '/floor/13' + + t.throws( + function () { fstream.error('candyman!', 'EBEES', true) }, + { + message: 'candyman!', + code: 'EBEES', + fstream_linkpath: '/floor/13', + fstream_class: 'Abstract' + } + ) + t.end() + }) + + t.end() +}) From a395f7e648511b497fd3942bafd827f481cc9bcf Mon Sep 17 00:00:00 2001 From: Forrest L Norvell Date: Thu, 28 Apr 2016 23:08:16 -0700 Subject: [PATCH 6/6] ci: bring version matrix up to date --- .travis.yml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index a092c82..da22d74 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,9 +1,11 @@ language: node_js node_js: - - iojs - - 0.12 - - 0.10 - - 0.8 + - "6" + - "4" + - "5" + - "0.12" + - "0.10" + - "0.8" before_install: - "npm config set spin false" - "npm install -g npm/npm"