Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Easier way to use memfs in tests #29

Closed
sapegin opened this issue Aug 30, 2017 · 7 comments
Closed

Easier way to use memfs in tests #29

sapegin opened this issue Aug 30, 2017 · 7 comments

Comments

@sapegin
Copy link
Contributor

sapegin commented Aug 30, 2017

I’m trying to use memfs in tests. Use case is like this:

  1. Mock fs with memfs:

    // __mocks__/fs.js
    process.chdir('/');
    module.exports = require('memfs').fs;
  2. Load the initial state from JSON using fromJSON method.

  3. Run methods I’m testing.

  4. Inspect the resulting JSON from toJSON.

The problem is that every time I call fromJSON it modifies the volume, keeping all existing files from previous tests.

I see two possible solutions:

  1. Recreate a memfs volume before each test. Seems a bit too complex.
  2. rm -rf / after each tests. But it seems scary — what if I forget to call jest.mock('fs')? ;-|

As a perfect solution I see fromJSON that can remove all existing files before loading JSON. Could be a new parameter or a new method.

What do you think?

@streamich
Copy link
Owner

streamich commented Aug 30, 2017

module.exports = require('memfs').fs;

You can also do module.exports = require('memfs');

Recreate a memfs volume before each test. Seems a bit too complex.

I would go with this one, just create new Volume for each test.

rm -rf / after each tests.

We could add rmrf method, like vol.rmrfSync('/');. Or just a method like .reset(), .truncate(), .clear().

As a perfect solution I see fromJSON that can remove all existing files before loading JSON.

The .fromJSON() method could be extended as follows, note reset: true:

vol.fromJSON(json, {
    cwd: '/', 
    reset: true,
    softlinks: { /* ... */ },
    hardlinks: { /* ... */ },
    fds: { /* ... open file state ... */ }
});

@sapegin
Copy link
Contributor Author

sapegin commented Aug 30, 2017

I would go with this one, just create new Volume for each test.

That’s inconvenient because I would need to export a method from this mock module that would recreate a volume.

Or just a method like .reset(), .truncate(), .clear().

I think that’s the most clear solution. I assume there is an easy way to implement that? I’d try ;-)

@streamich
Copy link
Owner

streamich commented Aug 30, 2017

I assume there is an easy way to implement that? I’d try ;-)

Hmm, I'm thinking just resetting these properties should do the job:

https://github.com/streamich/memfs/blob/master/src/volume.ts#L509-L539

Like:

this.ino = 0;
this.inodes = {};
this.releasedInos = [];
// etc.

And also create a new root node:

 this.root = new Link(this, null, '');
 this.root.setNode(this.createNode(true))

@streamich
Copy link
Owner

streamich commented Aug 30, 2017

BTW, could something like that do the job?

// __mocks__/fs.js
const {createFsFromVolume, Volume} = require('memfs');

process.chdir('/');

const beforeEach = () => {
    module.exports = createFsFromVolume(new Volume);
    module.exports.__beforeEach = beforeEach;
};

exports.__beforeEach = beforeEach;

And then:

beforeEach(() => {
    require('fs').__beforeEach();
});

@sapegin
Copy link
Contributor Author

sapegin commented Aug 30, 2017

I’m not sure there’s beforeEach inside mock files. And anyway I’ve submitter a PR with reset already ;-)

I’ve tried something similar and it didn’t work — I assume all tests still use a link to the initial volume and just overwriting module.exports isn’t enough.

@sapegin
Copy link
Contributor Author

sapegin commented Aug 30, 2017

I’m trying .reset() and test cases look really nice:

jest.mock('fs');

const { vol } = require('memfs');
const { copyFiles } = require('../fs');

afterEach(vol.reset.bind(vol));

describe('copyFiles()', () => {
	it('should copy a file', () => {
		vol.fromJSON({ 'tmpl/a': 'pizza' });

		copyFiles('tmpl', 'a');

		expect(vol.toJSON()).toMatchSnapshot();
	});

	it.skip('should copy multiple files', () => {
		vol.fromJSON({ 'tmpl/a': 'pizza', 'tmpl/b': 'coffee' });

		copyFiles('tmpl', ['a', 'b']);

		expect(vol.toJSON()).toMatchSnapshot();
	});

	it.skip('should not overwrite a file if  overwrite=false', () => {
		const json = { 'tmpl/a': 'pizza', a: 'pizza' };
		vol.fromJSON(json);

		copyFiles('tmpl', 'a', { overwrite: false });

		expect(vol.toJSON()).toEqual(json);
	});
});

@streamich
Copy link
Owner

I like the .toJSON() with .toMatchSnapshot().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants