From 772fdb0cd3163ad9299cdb1168b10059abe6ee71 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 4 Jul 2020 09:37:02 -0700 Subject: [PATCH] test: fix flaky test-fs-stream-construct The test is marked flaky on ARM because it times out on Raspberry Pi devices in CI. Split the single test file into four separate test files to ease debugging. Add fs.close() to avoid timing out. Fixes: https://github.com/nodejs/node/issues/33796 PR-URL: https://github.com/nodejs/node/pull/34203 Reviewed-By: Anna Henningsen Reviewed-By: Robert Nagy --- test/parallel/parallel.status | 2 - ...t-fs-stream-construct-compat-error-read.js | 32 +++ ...-fs-stream-construct-compat-error-write.js | 50 +++++ ...-fs-stream-construct-compat-graceful-fs.js | 70 ++++++ ...est-fs-stream-construct-compat-old-node.js | 97 ++++++++ test/parallel/test-fs-stream-construct.js | 212 ------------------ 6 files changed, 249 insertions(+), 214 deletions(-) create mode 100644 test/parallel/test-fs-stream-construct-compat-error-read.js create mode 100644 test/parallel/test-fs-stream-construct-compat-error-write.js create mode 100644 test/parallel/test-fs-stream-construct-compat-graceful-fs.js create mode 100644 test/parallel/test-fs-stream-construct-compat-old-node.js delete mode 100644 test/parallel/test-fs-stream-construct.js diff --git a/test/parallel/parallel.status b/test/parallel/parallel.status index 94d15064b38423..c6fd6fc178b510 100644 --- a/test/parallel/parallel.status +++ b/test/parallel/parallel.status @@ -40,8 +40,6 @@ test-async-hooks-http-parser-destroy: PASS,FLAKY # https://github.com/nodejs/node/pull/31178 test-crypto-dh-stateless: SKIP test-crypto-keygen: SKIP -# https://github.com/nodejs/node/issues/33796 -test-fs-stream-construct: PASS,FLAKY [$system==solaris] # Also applies to SmartOS diff --git a/test/parallel/test-fs-stream-construct-compat-error-read.js b/test/parallel/test-fs-stream-construct-compat-error-read.js new file mode 100644 index 00000000000000..0b7297a59f1bd7 --- /dev/null +++ b/test/parallel/test-fs-stream-construct-compat-error-read.js @@ -0,0 +1,32 @@ +'use strict'; + +const common = require('../common'); +const fs = require('fs'); +const assert = require('assert'); + +const tmpdir = require('../common/tmpdir'); +tmpdir.refresh(); + +{ + // Compat error. + + function ReadStream(...args) { + fs.ReadStream.call(this, ...args); + } + Object.setPrototypeOf(ReadStream.prototype, fs.ReadStream.prototype); + Object.setPrototypeOf(ReadStream, fs.ReadStream); + + ReadStream.prototype.open = common.mustCall(function ReadStream$open() { + const that = this; + fs.open(that.path, that.flags, that.mode, (err, fd) => { + that.emit('error', err); + }); + }); + + const r = new ReadStream('/doesnotexist', { emitClose: true }) + .on('error', common.mustCall((err) => { + assert.strictEqual(err.code, 'ENOENT'); + assert.strictEqual(r.destroyed, true); + r.on('close', common.mustCall()); + })); +} diff --git a/test/parallel/test-fs-stream-construct-compat-error-write.js b/test/parallel/test-fs-stream-construct-compat-error-write.js new file mode 100644 index 00000000000000..b47632c2c95e2f --- /dev/null +++ b/test/parallel/test-fs-stream-construct-compat-error-write.js @@ -0,0 +1,50 @@ +'use strict'; + +const common = require('../common'); +const fs = require('fs'); +const assert = require('assert'); + +const debuglog = (arg) => { + console.log(new Date().toLocaleString(), arg); +}; + +const tmpdir = require('../common/tmpdir'); +tmpdir.refresh(); + +{ + // Compat error. + debuglog('start test'); + + function WriteStream(...args) { + debuglog('WriteStream constructor'); + fs.WriteStream.call(this, ...args); + } + Object.setPrototypeOf(WriteStream.prototype, fs.WriteStream.prototype); + Object.setPrototypeOf(WriteStream, fs.WriteStream); + + WriteStream.prototype.open = common.mustCall(function WriteStream$open() { + debuglog('WriteStream open() callback'); + const that = this; + fs.open(that.path, that.flags, that.mode, (err, fd) => { + debuglog('inner fs open() callback'); + that.emit('error', err); + }); + }); + + fs.open(`${tmpdir.path}/dummy`, 'wx+', common.mustCall((err, fd) => { + debuglog('fs open() callback'); + assert.ifError(err); + fs.close(fd, () => { debuglog(`closed ${fd}`); }); + const w = new WriteStream(`${tmpdir.path}/dummy`, + { flags: 'wx+', emitClose: true }) + .on('error', common.mustCall((err) => { + debuglog('error event callback'); + assert.strictEqual(err.code, 'EEXIST'); + w.destroy(); + w.on('close', common.mustCall(() => { + debuglog('close event callback'); + })); + })); + })); + debuglog('waiting for callbacks'); +} diff --git a/test/parallel/test-fs-stream-construct-compat-graceful-fs.js b/test/parallel/test-fs-stream-construct-compat-graceful-fs.js new file mode 100644 index 00000000000000..ee1e00ed676042 --- /dev/null +++ b/test/parallel/test-fs-stream-construct-compat-graceful-fs.js @@ -0,0 +1,70 @@ +'use strict'; + +const common = require('../common'); +const fs = require('fs'); +const assert = require('assert'); +const fixtures = require('../common/fixtures'); + +const tmpdir = require('../common/tmpdir'); +tmpdir.refresh(); + +{ + // Compat with graceful-fs. + + function ReadStream(...args) { + fs.ReadStream.call(this, ...args); + } + Object.setPrototypeOf(ReadStream.prototype, fs.ReadStream.prototype); + Object.setPrototypeOf(ReadStream, fs.ReadStream); + + ReadStream.prototype.open = common.mustCall(function ReadStream$open() { + const that = this; + fs.open(that.path, that.flags, that.mode, (err, fd) => { + if (err) { + if (that.autoClose) + that.destroy(); + + that.emit('error', err); + } else { + that.fd = fd; + that.emit('open', fd); + that.read(); + } + }); + }); + + const r = new ReadStream(fixtures.path('x.txt')) + .on('open', common.mustCall((fd) => { + assert.strictEqual(fd, r.fd); + r.destroy(); + })); +} + +{ + // Compat with graceful-fs. + + function WriteStream(...args) { + fs.WriteStream.call(this, ...args); + } + Object.setPrototypeOf(WriteStream.prototype, fs.WriteStream.prototype); + Object.setPrototypeOf(WriteStream, fs.WriteStream); + + WriteStream.prototype.open = common.mustCall(function WriteStream$open() { + const that = this; + fs.open(that.path, that.flags, that.mode, function(err, fd) { + if (err) { + that.destroy(); + that.emit('error', err); + } else { + that.fd = fd; + that.emit('open', fd); + } + }); + }); + + const w = new WriteStream(`${tmpdir.path}/dummy`) + .on('open', common.mustCall((fd) => { + assert.strictEqual(fd, w.fd); + w.destroy(); + })); +} diff --git a/test/parallel/test-fs-stream-construct-compat-old-node.js b/test/parallel/test-fs-stream-construct-compat-old-node.js new file mode 100644 index 00000000000000..bd5aec689ff3b7 --- /dev/null +++ b/test/parallel/test-fs-stream-construct-compat-old-node.js @@ -0,0 +1,97 @@ +'use strict'; + +const common = require('../common'); +const fs = require('fs'); +const assert = require('assert'); +const fixtures = require('../common/fixtures'); + +const tmpdir = require('../common/tmpdir'); +tmpdir.refresh(); + +{ + // Compat with old node. + + function ReadStream(...args) { + fs.ReadStream.call(this, ...args); + } + Object.setPrototypeOf(ReadStream.prototype, fs.ReadStream.prototype); + Object.setPrototypeOf(ReadStream, fs.ReadStream); + + ReadStream.prototype.open = common.mustCall(function() { + fs.open(this.path, this.flags, this.mode, (er, fd) => { + if (er) { + if (this.autoClose) { + this.destroy(); + } + this.emit('error', er); + return; + } + + this.fd = fd; + this.emit('open', fd); + this.emit('ready'); + }); + }); + + let readyCalled = false; + let ticked = false; + const r = new ReadStream(fixtures.path('x.txt')) + .on('ready', common.mustCall(() => { + readyCalled = true; + // Make sure 'ready' is emitted in same tick as 'open'. + assert.strictEqual(ticked, false); + })) + .on('error', common.mustNotCall()) + .on('open', common.mustCall((fd) => { + process.nextTick(() => { + ticked = true; + r.destroy(); + }); + assert.strictEqual(readyCalled, false); + assert.strictEqual(fd, r.fd); + })); +} + +{ + // Compat with old node. + + function WriteStream(...args) { + fs.WriteStream.call(this, ...args); + } + Object.setPrototypeOf(WriteStream.prototype, fs.WriteStream.prototype); + Object.setPrototypeOf(WriteStream, fs.WriteStream); + + WriteStream.prototype.open = common.mustCall(function() { + fs.open(this.path, this.flags, this.mode, (er, fd) => { + if (er) { + if (this.autoClose) { + this.destroy(); + } + this.emit('error', er); + return; + } + + this.fd = fd; + this.emit('open', fd); + this.emit('ready'); + }); + }); + + let readyCalled = false; + let ticked = false; + const w = new WriteStream(`${tmpdir.path}/dummy`) + .on('ready', common.mustCall(() => { + readyCalled = true; + // Make sure 'ready' is emitted in same tick as 'open'. + assert.strictEqual(ticked, false); + })) + .on('error', common.mustNotCall()) + .on('open', common.mustCall((fd) => { + process.nextTick(() => { + ticked = true; + w.destroy(); + }); + assert.strictEqual(readyCalled, false); + assert.strictEqual(fd, w.fd); + })); +} diff --git a/test/parallel/test-fs-stream-construct.js b/test/parallel/test-fs-stream-construct.js deleted file mode 100644 index 4e687838f6b0c8..00000000000000 --- a/test/parallel/test-fs-stream-construct.js +++ /dev/null @@ -1,212 +0,0 @@ -'use strict'; - -const common = require('../common'); -const fs = require('fs'); -const assert = require('assert'); -const fixtures = require('../common/fixtures'); - -const tmpdir = require('../common/tmpdir'); -tmpdir.refresh(); - -const examplePath = fixtures.path('x.txt'); - -{ - // Compat with old node. - - function ReadStream(...args) { - fs.ReadStream.call(this, ...args); - } - Object.setPrototypeOf(ReadStream.prototype, fs.ReadStream.prototype); - Object.setPrototypeOf(ReadStream, fs.ReadStream); - - ReadStream.prototype.open = common.mustCall(function() { - fs.open(this.path, this.flags, this.mode, (er, fd) => { - if (er) { - if (this.autoClose) { - this.destroy(); - } - this.emit('error', er); - return; - } - - this.fd = fd; - this.emit('open', fd); - this.emit('ready'); - }); - }); - - let readyCalled = false; - let ticked = false; - const r = new ReadStream(examplePath) - .on('ready', common.mustCall(() => { - readyCalled = true; - // Make sure 'ready' is emitted in same tick as 'open'. - assert.strictEqual(ticked, false); - })) - .on('error', common.mustNotCall()) - .on('open', common.mustCall((fd) => { - process.nextTick(() => { - ticked = true; - r.destroy(); - }); - assert.strictEqual(readyCalled, false); - assert.strictEqual(fd, r.fd); - })); -} - -{ - // Compat with old node. - - function WriteStream(...args) { - fs.WriteStream.call(this, ...args); - } - Object.setPrototypeOf(WriteStream.prototype, fs.WriteStream.prototype); - Object.setPrototypeOf(WriteStream, fs.WriteStream); - - WriteStream.prototype.open = common.mustCall(function() { - fs.open(this.path, this.flags, this.mode, (er, fd) => { - if (er) { - if (this.autoClose) { - this.destroy(); - } - this.emit('error', er); - return; - } - - this.fd = fd; - this.emit('open', fd); - this.emit('ready'); - }); - }); - - let readyCalled = false; - let ticked = false; - const w = new WriteStream(`${tmpdir.path}/dummy`) - .on('ready', common.mustCall(() => { - readyCalled = true; - // Make sure 'ready' is emitted in same tick as 'open'. - assert.strictEqual(ticked, false); - })) - .on('error', common.mustNotCall()) - .on('open', common.mustCall((fd) => { - process.nextTick(() => { - ticked = true; - w.destroy(); - }); - assert.strictEqual(readyCalled, false); - assert.strictEqual(fd, w.fd); - })); -} - -{ - // Compat with graceful-fs. - - function ReadStream(...args) { - fs.ReadStream.call(this, ...args); - } - Object.setPrototypeOf(ReadStream.prototype, fs.ReadStream.prototype); - Object.setPrototypeOf(ReadStream, fs.ReadStream); - - ReadStream.prototype.open = common.mustCall(function ReadStream$open() { - const that = this; - fs.open(that.path, that.flags, that.mode, (err, fd) => { - if (err) { - if (that.autoClose) - that.destroy(); - - that.emit('error', err); - } else { - that.fd = fd; - that.emit('open', fd); - that.read(); - } - }); - }); - - const r = new ReadStream(examplePath) - .on('open', common.mustCall((fd) => { - assert.strictEqual(fd, r.fd); - r.destroy(); - })); -} - -{ - // Compat with graceful-fs. - - function WriteStream(...args) { - fs.WriteStream.call(this, ...args); - } - Object.setPrototypeOf(WriteStream.prototype, fs.WriteStream.prototype); - Object.setPrototypeOf(WriteStream, fs.WriteStream); - - WriteStream.prototype.open = common.mustCall(function WriteStream$open() { - const that = this; - fs.open(that.path, that.flags, that.mode, function(err, fd) { - if (err) { - that.destroy(); - that.emit('error', err); - } else { - that.fd = fd; - that.emit('open', fd); - } - }); - }); - - const w = new WriteStream(`${tmpdir.path}/dummy2`) - .on('open', common.mustCall((fd) => { - assert.strictEqual(fd, w.fd); - w.destroy(); - })); -} - -{ - // Compat error. - - function ReadStream(...args) { - fs.ReadStream.call(this, ...args); - } - Object.setPrototypeOf(ReadStream.prototype, fs.ReadStream.prototype); - Object.setPrototypeOf(ReadStream, fs.ReadStream); - - ReadStream.prototype.open = common.mustCall(function ReadStream$open() { - const that = this; - fs.open(that.path, that.flags, that.mode, (err, fd) => { - that.emit('error', err); - }); - }); - - const r = new ReadStream('/doesnotexist', { emitClose: true }) - .on('error', common.mustCall((err) => { - assert.strictEqual(err.code, 'ENOENT'); - assert.strictEqual(r.destroyed, true); - r.on('close', common.mustCall()); - })); -} - -{ - // Compat error. - - function WriteStream(...args) { - fs.WriteStream.call(this, ...args); - } - Object.setPrototypeOf(WriteStream.prototype, fs.WriteStream.prototype); - Object.setPrototypeOf(WriteStream, fs.WriteStream); - - WriteStream.prototype.open = common.mustCall(function WriteStream$open() { - const that = this; - fs.open(that.path, that.flags, that.mode, (err, fd) => { - that.emit('error', err); - }); - }); - - fs.open(`${tmpdir.path}/dummy3`, 'wx+', common.mustCall((err) => { - assert(!err); - const w = new WriteStream(`${tmpdir.path}/dummy3`, - { flags: 'wx+', emitClose: true }) - .on('error', common.mustCall((err) => { - assert.strictEqual(err.code, 'EEXIST'); - w.destroy(); - w.on('close', common.mustCall()); - })); - })); -}