Repository navigation
Mocking of fs module in Jest broken in 10.5.0 #21489
Description
- Version:
v10.5.0 - Platform:
Darwin Simens-MBP 17.4.0 Darwin Kernel Version 17.4.0: Sun Dec 17 09:19:54 PST 2017; root:xnu-4570.41.2~1/RELEASE_X86_64 x86_64 - Subsystem:
fs
With the following file:
jest.mock('fs');
const mockedFs = require('fs');
console.log(typeof mockedFs.ReadStream);
console.log(typeof mockedFs.WriteStream);
test('empty test to have Jest run the file', () => {
expect(true).toBe(true);
});It prints the following in Node 10.4.1:
● Console
console.log test.js:5
function
console.log test.js:6
function
And the following in Node 10.5.0:
● Console
console.log test.js:5
undefined
console.log test.js:6
undefined
More concretely, the following is missing (File){Read,Write}Stream in 10.5.0, while they're present in 10.4.1:
const fs = require('fs');
const jestMock = require('jest-mock');
const {members: mockMembers} = jestMock.getMetadata(fs);
console.log(mockMembers);Source of that function: https://github.com/facebook/jest/blob/24df0751a818e570fe74dd0d73285a63e322d37f/packages/jest-mock/src/index.js#L642-L723
Probably related to #20764.
I'll add that this should probably be addressed in Jest, but this is potentially pretty breaking for consumers of Jest (of which not everybody will upgrade to the latest version).
In particular, graceful-fs completely breaks if required after a jest.mock('fs');
jest.mock('fs');
require('graceful-fs');Explodes with
TypeError: Cannot read property 'prototype' of undefined
1 | jest.mock('fs');
2 |
> 3 | require('graceful-fs');
4 |
at patch (node_modules/graceful-fs/graceful-fs.js:166:54)
at Object.<anonymous> (ugh.test.js:3:1)
Activity
vitkarpov commentedon Jun 23, 2018
Seems in v10.5.0
ReadStreamandWriteStreambecame getters with lazy loading instead of regular properties.In Jest
getSlotsmethod passes getters, that's why this props are not included eventually. @SimenB Probably you know the reason why getters not included in the first place.SimenB commentedon Jun 30, 2018
@jasnell Sorry to ping you directly, but are there any plans for a fix here?
jasnell commentedon Jul 12, 2018
@SimenB ... my apologies for just seeing this, I've been on vacation and have a serious backlog of issues to catch up on. It appears that @targos has a potential fix in the works.
SimenB commentedon Jul 13, 2018
Dug into why we don't include accessors, and it seems to be by design: jestjs/jest#390
I'll try to ask if it's still needed
targos commentedon Jul 13, 2018
Our accessors are enumerable:
SimenB commentedon Jul 13, 2018
Yeah, the PR is poorly named. If you look at the code it uses
Object.getOwnPropertyDescriptorand bails if it's a getter, it doesn't actually care aboutenumerableif it's a getter, which isn't mentioned.vitkarpov commentedon Jul 13, 2018
And now enumerability check is gone but it was there in the first place 🤔 Seems like unexpected disappearing to me.
SimenB commentedon Jul 13, 2018
Good point, it would've worked if we'd have kept the enumerable check, I read that too quickly...
SimenB commentedon Jul 13, 2018
jestjs/jest#409, removed without any context provided... I've asked, but haven't received a response yet
vitkarpov commentedon Jul 13, 2018
@SimenB I assume you've asked within some private channel, could you, please, provide an update here? I'm intrigued 😃
fs: stop lazy loading stream constructors
fs: stop lazy loading stream constructors
targos commentedon Jul 19, 2018
The breaking change is reverted on Node 10 (will be released with the next version)
fix: revert "fs: stop lazy loading stream constructors"