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

Implicit createCwd even with createCwd: false #100

Open
HatiGamedev opened this issue Mar 29, 2016 · 9 comments
Open

Implicit createCwd even with createCwd: false #100

HatiGamedev opened this issue Mar 29, 2016 · 9 comments

Comments

@HatiGamedev
Copy link

While tinkering with mountfs and mock-fs combination, I discovered a weird behaviour of mock-fs.

Following code will create the process.cwd() even if not so desired by using the option.createCwd = false

let mockfs = ...
const dummyFS = {
  'foo': 'bar'
}

mockfs(dummyFS, {createCwd: false});

Currently my solution is using a root directory - but that's one extra indentation :/

let mockfs = ...
const dummyFS = {
 '/data': {
  'foo': 'bar'
 }  
}

mockfs(dummyFS, {createCwd: false});

This behavior seems counter intuitive, especially if you want to use the mockfs#fs functionality.

@tschaub
Copy link
Owner

tschaub commented Apr 1, 2016

You should be able to provide the absolute path to the file you want to create. E.g.

mock({'/data/foo': 'bar'}, {createCwd: false});

What would you expect the absolute path to your foo file to be in the first case above?

@HatiGamedev
Copy link
Author

It simply should not exist. Because there is no root relation.
createCwd does magic here. Placing an file not supposed to be in ~/

The main issue arises if you use MockFs#fs function. Which you could mount using MountFs. Where 'foo' from Example#1 should exist in mount.

@tschaub
Copy link
Owner

tschaub commented Apr 2, 2016

@HatiEth can you provide a test that demonstrates the behavior you expect?

I agree there could be something that needs fixing. It would be good to get more specific about the expected behavior.

@HatiGamedev
Copy link
Author

Yeah, mostly what I ment before (wrote it on phone...):

createCwd does not implicitly states that it will also be used as such.

So adding another placeInsideCwd or something would be good option? To keep old behaviour?

Test code: (discussable, just wrote down right now)

'use strict';

const should = require('should');
const fs = require('fs');
const mountFS = require('mountfs');
const mockfs = require('mock-fs');

mountFS.patchInPlace();

const dummyFS = {
  '/': {
    'file': 'some random file content'
  },
  '/directly_parented': 'directly_parented content',
  'unparent_file': 'unparent_file important content'
};

let cwd = process.cwd();

describe.only('mockfs#fs', function() {
  before(function() {
    let mock = mockfs.fs(dummyFS, { createCwd: false, createTmp: false });
    fs.mount('/mocks', mock);
    /*
    let files = fs.readdirSync('/mocks');
    console.log(files);
    */
  });
  after(function() {
    fs.unmount('/mocks'); // unmock '/mocks...' fs
    fs.unpatch(); // unpatch patched fs
  });

  /**
   * Should exists, but would be mountfs issue.
   */
  it('/mocks exists', function(done) {
    fs.stat('/mocks', (err, stats) => {
      should.not.exist(err);
      stats.isDirectory().should.be.true();
      done();
    });
  });

  /**
   * Make sure cwd is not created as asked. (createCwd: false)
   */
  it('cwd does not exists', function(done) {
    fs.stat(cwd, (err, stats) => {
      should.not.exist(err);
      stats.isDirectory().should.be.false();
      done();
    });
  });

  describe('/mocks/file', function() {
    /**
     * Should exists because in root dir? => but '/' gets converted to undefined
     */
    it('exists', function(done) {
      fs.stat('/mocks/file', (err, stats) => {
        should.not.exist(err);
        stats.isFile().should.be.true();
        done();
      });
    });

    it('has expected content');
  });

  /**
   * Expected to exists but is in ${cwd}/unparent_file
   */
  describe('/mocks/unparent_file', function() {
    it('exists', function(done) {
      fs.stat('/mocks/unparent_file', (err, stats) => {
        should.not.exist(err);
        stats.isFile().should.be.true();
        done();
      });
    });
    it('has expected content');
  });

  describe('/mocks/directly_parented', function() {
    it('exists', function(done) {
      fs.stat('/mocks/directly_parented', (err, stats) => {
        should.not.exist(err);
        stats.isFile().should.be.true();
        done();
      });
    });
    it('has expected content');
  });

  describe('cwd', function() {
    it('pure should exists, because mounted to /mocks not /', function(done) {
      fs.stat(`${cwd}`, (err, stats) => {
        should.not.exist(err);
        stats.isDirectory().should.be.true();
        done();
      });
    });
    it('/mocks/${cwd} should not exist, explicit said no createCwd', function(done) {
      fs.stat(`/mocks${cwd}`, (err, stats) => {
        should.not.exist(err);
        stats.isDirectory().should.be.false();
        done();
      });
    });
  });
});

@gyandeeps
Copy link

Same thing happened to me:

        finalMockDir = {
             "eslint/fixtures/config-hierarchy": {}
         };
        mockFs(finalMockDir, {
            createCwd: false,
            createTmp: true
        });

This always creates the above structure inside my process.cwd() but doesnt not create it inside os.tmpdir().

I think the issue is here: https://github.com/tschaub/mock-fs/blob/master/lib/filesystem.js#L15
This always resolves it according to process.cwd()

@tschaub
Copy link
Owner

tschaub commented May 24, 2016

It is still not clear to me what you would expect from this code:

var mock = require('mock-fs');
mock({foo: {}}, {createCwd: false});

What sort of assertion would you expect to be able to make after this?

@gyandeeps
Copy link

finalMockDir = {
             "eslint/fixtures/config-hierarchy": {}
         };
        mockFs(finalMockDir, {
            createCwd: false,
            createTmp: true
        });

Lets say if I execute the above code, then I should be able to say

fs.readdirSync(path.join(os.tmpdir(), "eslint")); // -> should not throw error. currently this will throw error.

@tschaub
Copy link
Owner

tschaub commented Jul 10, 2016

@gyandeeps - you'd need to provide the full path to eslint in your mock config. For example, this currently works:

var assert = require('assert');
var path = require('path');
var mock = require('mock-fs');
var fs = require('fs');
var os = require('os');

var dir = path.join(os.tmpdir(), 'new-directory');
assert.throws(() => fs.accessSync(dir));

mock({
  [dir]: {}
});
assert.doesNotThrow(() => fs.accessSync(dir));

mock.restore();
assert.throws(() => fs.accessSync(dir));

Using just those same modules (i.e. no mocha, no mountfs, etc.), I'm still not clear what someone would expect to be able to assert after doing this:

mock({foo: 'bar'}, {createCwd: false});
// what would you be able to assert here?

Currently, you can assert this:

assert.doesNotThrow(() => fs.accessSync(path.resolve('foo')));

But it sounds like @HatiEth might expect that to throw instead.

@HatiGamedev
Copy link
Author

mock({foo: 'bar'}, {createCwd: false});
// what would you be able to assert here?

Well what would be the most obvious result to expect? The file is inaccessible.
If the convention says, {'pure-file': 'content'} with pure-file being only a direct file path ( no / or \ ) - more like ./ then I would say the file is simply inaccessible because I expicitly said not to create the Cwd

I would not expect anything thrown on your side, if it is documented properly. But on fs side i would expect an error to be risen - because the file is destined to not exist.

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

3 participants