4

I am trying to test a library function that I have written (it works in my code) but cannot get the testing with the mock of the fs to work. I have a series of functions for working with the OS wrapped in functions so different parts of the application can use the same calls.

I have tried to follow this question with mocking the file system, but it does not seem to work for me.

A short sample to demonstrate the basics of my issue are below:

import * as fs from 'fs';
export function ReadFileContentsSync(PathAndFileName:string):string {
    if (PathAndFileName === undefined || PathAndFileName === null || PathAndFileName.length === 0) {
        throw new Error('Need a Path and File');
    }
    return fs.readFileSync(PathAndFileName).toString();
}

So now I am trying to test this function using Jest:

import { ReadFileContentsSync } from "./read-file-contents-sync";
const fs = require('fs');

describe('Return Mock data to test the function', () => {
    it('should return the test data', () => {
        const TestData:string = 'This is sample Test Data';

// Trying to mock the reading of the file to simply use TestData
        fs.readFileSync = jest.fn();                
        fs.readFileSync.mockReturnValue(TestData);

// Does not need to exist due to mock above     
        const ReadData = ReadFileContentsSync('test-path');
        expect(fs.readFileSync).toHaveBeenCalled();
        expect(ReadData).toBe(TestData);
    });
});

I get an exception that the file does not exist, but I would have expected the actual call to fs.readFileSync to not have been called, but the jest.fn() mock to have been used.

ENOENT: no such file or directory, open 'test-path'

I am not sure how to do this mock?

Steven Scott
  • 10,234
  • 9
  • 69
  • 117
  • Try enabling `esModuleInterop` in your `tsconfig` and use `import fs from 'fs'` and try agin. The `import * as fs ...` is lilkely doing a copy thus your direct mock won't work. – unional Sep 28 '18 at 17:52
  • And you can mock using the lib mock in `jest` instead of mocking a function. Alternatively, consider functional programming pattern/OO to manage your dependencies. – unional Sep 28 '18 at 17:54
  • https://jestjs.io/docs/en/bypassing-module-mocks – unional Sep 28 '18 at 17:54
  • @unional I tried your suggestion to enable the esModuleInterop, and then the `import fs from 'fs'`, the mock did not happen though, as the mock data was not returned, but an error from the attempt to access a file that clearly does not exist. `ENOENT: no such file or directory, open test-path` – Steven Scott Oct 01 '18 at 00:08
  • Then you have to use the jest bypass mock. Personally I’m not a friend of it. I’d rather follow functional programming conventions. – unional Oct 01 '18 at 01:29

2 Answers2

7

Since I mentioned about functional / OO / and the dislike of jest mock, I feel like I should fill in some explanation here.

I'm not against jest.mock() or any mocking library (such as sinon). I have used them before, and they definitely serve their purpose and is a useful tool. But I find myself do not need them for the most part, and there is some tradeoff when using them.

Let me first demonstrate three ways that the code can be implemented without the use of mock.

The first way is functional, using a context as the first argument:

// read-file-contents-sync.ts
import fs from 'fs';
export function ReadFileContentsSync({ fs } = { fs }, PathAndFileName: string): string {
    if (PathAndFileName === undefined || PathAndFileName === null || PathAndFileName.length === 0) {
        throw new Error('Need a Path and File');
    }
    return fs.readFileSync(PathAndFileName).toString();
}

// read-file-contents-sync.spec.ts
import { ReadFileContentsSync } from "./read-file-contents-sync";

describe('Return Mock data to test the function', () => {
    it('should return the test data', () => {
        const TestData:Buffer = new Buffer('This is sample Test Data');

        // Trying to mock the reading of the file to simply use TestData
        const fs = {
            readFileSync: () => TestData
        }

        // Does not need to exist due to mock above     
        const ReadData = ReadFileContentsSync({ fs }, 'test-path');
        expect(ReadData).toBe(TestData.toString());
    });
});

The second way is to use OO:

// read-file-contents-sync.ts
import fs from 'fs';
export class FileReader {
    fs = fs
    ReadFileContentsSync(PathAndFileName: string) {
        if (PathAndFileName === undefined || PathAndFileName === null || PathAndFileName.length === 0) {
            throw new Error('Need a Path and File');
        }
        return this.fs.readFileSync(PathAndFileName).toString();
    }
}

// read-file-contents-sync.spec.ts
import { FileReader } from "./read-file-contents-sync";

describe('Return Mock data to test the function', () => {
    it('should return the test data', () => {
        const TestData: Buffer = new Buffer('This is sample Test Data');

        const subject = new FileReader()
        subject.fs = { readFileSync: () => TestData } as any

        // Does not need to exist due to mock above     
        const ReadData = subject.ReadFileContentsSync('test-path');
        expect(ReadData).toBe(TestData.toString());
    });
});

The third way uses a modified functional style, which requires TypeScript 3.1 (technically you can do that prior to 3.1, but it is just a bit more clumsy involving namespace hack):

// read-file-contents-sync.ts
import fs from 'fs';
export function ReadFileContentsSync(PathAndFileName: string): string {
    if (PathAndFileName === undefined || PathAndFileName === null || PathAndFileName.length === 0) {
        throw new Error('Need a Path and File');
    }
    return ReadFileContentsSync.fs.readFileSync(PathAndFileName).toString();
}
ReadFileContentsSync.fs = fs

// read-file-contents-sync.spec.ts
import { ReadFileContentsSync } from "./read-file-contents-sync";

describe('Return Mock data to test the function', () => {
    it('should return the test data', () => {
        const TestData: Buffer = new Buffer('This is sample Test Data');

        // Trying to mock the reading of the file to simply use TestData
        ReadFileContentsSync.fs = {
            readFileSync: () => TestData
        } as any

        // Does not need to exist due to mock above     
        const ReadData = ReadFileContentsSync('test-path');
        expect(ReadData).toBe(TestData.toString());
    });
});

The first two ways provide more flexibility and isolation because each call/instance have their own reference of the dependency. This means there will be no way that the "mock" of one test would affect the other.

The third way does not prevent that from happening but have the benefit of not changing the signature of the original function.

The bottom of all these is dependency management. Most of the time a program or code is hard to maintain, use, or test is because it does not provide a way for the calling context to control the dependency of its callee.

Relying on mocking library (especially a mocking system as powerful as jest.mock()) can easily get to a habit of ignoring this important aspect.

One nice article I would recommend everyone to check out is Uncle Bob's Clean Architecture: https://8thlight.com/blog/uncle-bob/2012/08/13/the-clean-architecture.html

unional
  • 14,651
  • 5
  • 32
  • 56
  • I liked the answers, but had some notes/questions: #1 `export function ReadFileContentsSync({ fs } = { fs }...` the first parameter here would mean all our calling code would have to import fs as well, since it needs to be passed to the library function. If we changed to a different handler than fs, that would involve a lot of changes. For #2, I think this is better choice, except the fs is public and could be overwritten in a call to the library to use something different (needed for testing) than what is desired as we are trying to use 1 library only but code review should catch issues. – Steven Scott Oct 01 '18 at 21:31
  • 1
    For #1, yes, your calling code would need to have it passed in. In essence, these "external dependency" should be managed by the "main boundary", i.e. the application. For #2, not sure what do you mean. Can you elaborate? Overall, what you said is true. It is public. My argument for that is "all properties in JavaScript" are public already. IMO sometimes we take encapsulation too seriously. We should know what can and can not do, but not necessary restrict it to a point that makes it hard/impossible to do our work efficiently. – unional Oct 01 '18 at 22:32
  • And about the "main boundary", that's one reason a good DI solution should be available or even baked in to the language IMO. It is a hassle to carry all the dependencies your code needed and that can easily violate basic design principles (low level detail changes causes high level policy to change). The question is learn to use DI effectively and correctly. That can solve a whole school of issues. – unional Oct 01 '18 at 22:36
  • It is more we have a mix of junior, intermediate and senior developers. The more junior, the more likely to change something that is public, that they can change without realizing the impact. Code Reviews can help track this and educate the juniors (and intermediates) but we might already have code committed causing problems. However, I think the class with the dependency injection is the best way and I am going to try to get our library changed. – Steven Scott Oct 02 '18 at 16:58
  • One thing of note in these however, is Typescript does like to give the error `error TS1192: Module '"fs"' has no default export.`, unless you enable the `esModuleInterop` setting in the tsconfig.json. – Steven Scott Oct 02 '18 at 17:01
  • This is working well, but how would you wrap the accessSync (void function checking a file exists with proper permissions)? – Steven Scott Oct 02 '18 at 17:51
  • 1
    What do you mean by "wrap"? Do you mean how to stub it? `fs = { accessSync() { throw ... } }` or `fs = { accessSync() { } }`? – unional Oct 02 '18 at 19:24
  • Looking to how to actually add in the call without doing anything, and without generating TSLint errors, etc. – Steven Scott Oct 03 '18 at 15:01
  • What I was wondering was how to do the accessSync, similar to your readFileSync with setting the different parameter options for testing (Read Only, Admin Privilege, etc.) and have the required properties (rename) included? Maybe best to post into another question? – Steven Scott Oct 08 '18 at 23:49
  • Seems like another question is a good idea. Still not that clear about what you try to do with `accessSync()`. – unional Oct 09 '18 at 01:08
  • I added the new question: https://stackoverflow.com/questions/52725339/testing-a-fs-library-function-with-jest-typescript-and-a-dummy-function – Steven Scott Oct 09 '18 at 16:18
1

While unional's comment helped to point me in the right direction, the import for the fs, was done in my code as import * as fs from 'fs'. This seemed to be the problem. Changing the import here to simply be import fs from 'fs' and that solved the problem.

Therefore, the code becomes:

import fs from 'fs';
export function ReadFileContentsSync(PathAndFileName:string):string {
    if (PathAndFileName === undefined || PathAndFileName === null || PathAndFileName.length === 0) {
        throw new Error('Need a Path and File');
    }
    return fs.readFileSync(PathAndFileName).toString();
}

And the test file:

jest.mock('fs');
import { ReadFileContentsSync } from "./read-file-contents-sync";

import fs from 'fs';

describe('Return Mock data to test the function', () => {
    it('should return the test data', () => {
        const TestData:Buffer = new Buffer('This is sample Test Data');

// Trying to mock the reading of the file to simply use TestData
        fs.readFileSync = jest.fn();                
        fs.readFileSync.mockReturnValue(TestData);

// Does not need to exist due to mock above     
        const ReadData = ReadFileContentsSync('test-path');
        expect(fs.readFileSync).toHaveBeenCalled();
        expect(ReadData).toBe(TestData.toString());
    });
});
Steven Scott
  • 10,234
  • 9
  • 69
  • 117