Skip to content

Commit 7667838

Browse files
Micha Reiserfacebook-github-bot
authored andcommitted
Fix worker-test and fs.createWriteStream spec incompliance
Summary: The CircleCI build of metro is currently failing because `jest` fails to mock the `console` module. This is because the typeof console changed in the latest node version ([PR](nodejs/node#35399)). Meaning that the typeof `console` is `console` and Jest doesn't know how to mock a value of type `console`. This diff manually mocks the `console` object to avoid this issue (verified with using Node 15 locally). However, the tests then started failing because the `close` event of streams was emitted more than once by our memory fs implementation. The issue is that our `createWriteStream` implementetion does not specify the `emitClose: false` when creating the stream as required according to the [docs](https://nodejs.org/api/fs.html#fs_fs_createwritestream_path_options): > By default, the stream will not emit a 'close' event after it has been destroyed. This is the opposite of the default for other Writable streams. Set the emitClose option to true to change this behavior. I changed that in our `memory-fs` implementation and removed the manual emit of the `close` event. I then had to change the tests because the order of the events is `end`, `finish`, `close` and the tests asserted in the `end` event. The tests then started passing on Node 15 but, surprise, they now failed on Node 12... The reason is that the [`stream.Writeable`](https://nodejs.org/docs/latest-v13.x/api/stream.html#stream_constructor_new_stream_writable_options) option `autoDestroy` has changed with Node13 from default false to default true. Hardcoding `default: true` finally makes all tests passing on Node 12 - 15. Reviewed By: cpojer Differential Revision: D25495034 fbshipit-source-id: fdf871fa4dfbec079bc9ec1843fcff7facb3e0be
1 parent 663621c commit 7667838

3 files changed

Lines changed: 19 additions & 17 deletions

File tree

packages/buck-worker-tool/src/__tests__/worker-test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ describe('Buck worker:', () => {
124124
fs.createWriteStream = (...args) => {
125125
const writeStream = createWriteStreamImpl(...args);
126126
++openedStreams;
127-
writeStream.on('close', () => --openedStreams);
127+
writeStream.on('finish', () => --openedStreams);
128128
return writeStream;
129129
};
130130
});

packages/metro-memory-fs/src/__tests__/index-test.js

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -103,32 +103,32 @@ describe('posix support', () => {
103103

104104
describe('createWriteStream', () => {
105105
it('writes a file', done => {
106-
const st = fs.createWriteStream('/foo.txt');
106+
const st = fs.createWriteStream('/foo.txt', {emitClose: true});
107107
let opened = false;
108108
let closed = false;
109109
st.on('open', () => (opened = true));
110110
st.on('close', () => (closed = true));
111111
st.write('test');
112112
st.write(' foo');
113-
st.end(() => {
113+
st.end();
114+
115+
st.on('close', () => {
114116
expect(opened).toBe(true);
115117
expect(closed).toBe(true);
116118
expect(fs.readFileSync('/foo.txt', 'utf8')).toEqual('test foo');
119+
117120
done();
118121
});
119122
});
120123

121124
it('writes a file, as buffer', done => {
122125
const st = fs.createWriteStream('/foo.txt');
123126
let opened = false;
124-
let closed = false;
125127
st.on('open', () => (opened = true));
126-
st.on('close', () => (closed = true));
127128
st.write(Buffer.from('test'));
128129
st.write(Buffer.from(' foo'));
129130
st.end(() => {
130131
expect(opened).toBe(true);
131-
expect(closed).toBe(true);
132132
expect(fs.readFileSync('/foo.txt', 'utf8')).toEqual('test foo');
133133
done();
134134
});
@@ -138,13 +138,10 @@ describe('posix support', () => {
138138
fs.writeFileSync('/foo.txt', 'test bar');
139139
const st = fs.createWriteStream('/foo.txt', {start: 5, flags: 'r+'});
140140
let opened = false;
141-
let closed = false;
142141
st.on('open', () => (opened = true));
143-
st.on('close', () => (closed = true));
144142
st.write('beep');
145143
st.end(() => {
146144
expect(opened).toBe(true);
147-
expect(closed).toBe(true);
148145
expect(fs.readFileSync('/foo.txt', 'utf8')).toEqual('test beep');
149146
done();
150147
});
@@ -154,13 +151,10 @@ describe('posix support', () => {
154151
const fd = fs.openSync('/bar.txt', 'w');
155152
const st = fs.createWriteStream('/foo.txt', {fd});
156153
let opened = false;
157-
let closed = false;
158154
st.on('open', () => (opened = true));
159-
st.on('close', () => (closed = true));
160155
st.write('beep boop');
161156
st.end(() => {
162157
expect(opened).toBe(false);
163-
expect(closed).toBe(true);
164158
expect(fs.readFileSync('/bar.txt', 'utf8')).toEqual('beep boop');
165159
done();
166160
});

packages/metro-memory-fs/src/index.js

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1027,6 +1027,7 @@ class MemoryFs {
10271027
flags?: string,
10281028
mode?: number,
10291029
start?: number,
1030+
emitClose?: boolean,
10301031
...
10311032
}
10321033
| Encoding,
@@ -1037,30 +1038,36 @@ class MemoryFs {
10371038
autoClose?: boolean,
10381039
encoding?: Encoding,
10391040
fd?: ?number,
1041+
emitClose?: boolean,
10401042
flags?: string,
10411043
mode?: number,
10421044
start?: number,
10431045
...
10441046
}
10451047
| Encoding,
10461048
) => {
1047-
let autoClose, fd, flags, mode, start;
1049+
let autoClose, fd, flags, mode, start, emitClose;
10481050
if (typeof options !== 'string' && options != null) {
1049-
({autoClose, fd, flags, mode, start} = options);
1051+
({autoClose, fd, flags, mode, start, emitClose} = options);
10501052
}
10511053
let st = null;
10521054
if (fd == null) {
10531055
fd = this._open(pathStr(filePath), flags || 'w', mode);
10541056
process.nextTick(() => (st: any).emit('open', fd));
10551057
}
10561058
const ffd = fd;
1057-
const ropt = {fd, writeSync: this._write.bind(this), filePath, start};
1059+
const ropt = {
1060+
fd,
1061+
writeSync: this._write.bind(this),
1062+
filePath,
1063+
start,
1064+
emitClose: emitClose ?? false,
1065+
};
10581066
const rst = new WriteFileStream(ropt);
10591067
st = rst;
10601068
if (autoClose !== false) {
10611069
const doClose = () => {
10621070
this.closeSync(ffd);
1063-
rst.emit('close');
10641071
};
10651072
rst.on('finish', doClose);
10661073
rst.on('error', doClose);
@@ -1570,9 +1577,10 @@ class WriteFileStream extends stream.Writable {
15701577
filePath: FilePath,
15711578
writeSync: WriteSync,
15721579
start?: number,
1580+
emitClose?: boolean,
15731581
...
15741582
}) {
1575-
super();
1583+
super({emitClose: opts.emitClose, autoDestroy: true});
15761584
this.path = opts.filePath;
15771585
this.bytesWritten = 0;
15781586
this._fd = opts.fd;

0 commit comments

Comments
 (0)