Skip to content

Commit

Permalink
test: fix flaky test-fs-stream-construct
Browse files Browse the repository at this point in the history
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: #33796

PR-URL: #34203
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
  • Loading branch information
Trott committed Jul 7, 2020
1 parent ee3416b commit 772fdb0
Show file tree
Hide file tree
Showing 6 changed files with 249 additions and 214 deletions.
2 changes: 0 additions & 2 deletions test/parallel/parallel.status
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
32 changes: 32 additions & 0 deletions test/parallel/test-fs-stream-construct-compat-error-read.js
Original file line number Diff line number Diff line change
@@ -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());
}));
}
50 changes: 50 additions & 0 deletions test/parallel/test-fs-stream-construct-compat-error-write.js
Original file line number Diff line number Diff line change
@@ -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');
}
70 changes: 70 additions & 0 deletions test/parallel/test-fs-stream-construct-compat-graceful-fs.js
Original file line number Diff line number Diff line change
@@ -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();
}));
}
97 changes: 97 additions & 0 deletions test/parallel/test-fs-stream-construct-compat-old-node.js
Original file line number Diff line number Diff line change
@@ -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);
}));
}
Loading

0 comments on commit 772fdb0

Please sign in to comment.