0

In trying to test a library function that utilized the fs module, I was given help in this question to better test the functionality, without the mocks, which I agreed with @unional was a better method.

I am trying to do the same with the accessSync method, but it is not working the same way and needs some changes for testing.

My code, following the changes suggested by @unional:

import fs from 'fs';
export function AccessFileSync(PathAndFileName: string):boolean {
    if (PathAndFileName === undefined || PathAndFileName === null || PathAndFileName.length === 0) {
        throw new Error('Missing File Name');
    }
    try {
        AccessFileSync.fs.accessSync(PathAndFileName, fs.constants.F_OK | fs.constants.R_OK);
    } catch {
        return false;
    }
    return true;
}
AccessFileSync.fs = fs;

Now, to try to test it, I would:

describe('Return Mock data to test the function', () => {
    it('should return the test data', () => {
        // mock function
        AccessFileSync.fs = {
            accessSync: () => { return true; }
        } as any;

        const AccessAllowed:boolean = AccessFileSync('test-path');      // Does not need to exist due to mock above
        expect(AccessFileSync.fs.accessSync).toHaveBeenCalled();
        expect(AccessAllowed).toBeTruthy();
    });
});

This does work for the first test, but subsequent tests, changing the test, does not get the new value. For instance:

describe('Return Mock data to test the function', () => {
    it('should return the test data', () => {
        // mock function
        AccessFileSync.fs = {
            accessSync: () => { return true; }
        } as any;

        const AccessAllowed:boolean = AccessFileSync('test-path');      // Does not need to exist due to mock above
        expect(AccessFileSync.fs.accessSync).toHaveBeenCalled();
        expect(AccessAllowed).toBeTruthy();
    });
});
describe('Return Mock data to test the function', () => {
    it('should return the test data', () => {
        // mock function
        AccessFileSync.fs = {
            accessSync: () => { return false; }
        } as any;

        const AccessAllowed:boolean = AccessFileSync('test-path');      // Does not need to exist due to mock above
        expect(AccessFileSync.fs.accessSync).toHaveBeenCalled();
        expect(AccessAllowed).toBeFalsy();  // <- This Fails
    });
});

Also, I would like to have the tslint pass, which does not like the as any layout, and would prefer the Variable:type notation.

Steven Scott
  • 10,234
  • 9
  • 69
  • 117

1 Answers1

1

Your code never returns false:

import fs from 'fs';
export function AccessFileSync(PathAndFileName: string): boolean {
  if (PathAndFileName === undefined || PathAndFileName === null || PathAndFileName.length === 0) {
    throw new Error('Missing File Name');
  }
  try {
    AccessFileSync.fs.accessSync(PathAndFileName, fs.constants.F_OK | fs.constants.R_OK);
  }
  catch {
    return false; // here
  }
  return true;
}
AccessFileSync.fs = fs;

Also, your stub needs to throw in order to simulate the same behavior.

describe('Return Mock data to test the function', () => {
  it('should return the test data', () => {
    // mock function
    AccessFileSync.fs = {
      accessSync: () => { throw new Error('try to mimic the actual error') }
    };

    const AccessAllowed: boolean = AccessFileSync('test-path');
    expect(AccessAllowed).toBeFalsy(); 
  });
});

As for the lint error, there are two ways to handle it.

The first one is type assertion, which is what you do, and you can cast it to anything you want, such as as typeof fs, but personally I would think that is overkill.

Type assertion should generally be discouraged as it just tells the compiler, "hey, I know you thought x is X, but I know it is actually Y, so let's treat it as Y", so you basically lost the benefit of type checking.

But specifically for mock and stub, it is ok because you are consciously aware that you are "faking" it, and you have the test to back up the lost of type checking.

The second way involves the Interface Segregation Principle (ISP, The I in SOLID principles).

The idea is to asks for what you actually need, instead of acquiring the whole type/interface.

AccessFileSync.fs = fs as Pick<typeof fs, 'accessSync'>

Your test doesn't need to do the type assertion anymore.

Note that I have to use type assertion here because X.y: <type> = value is not a valid syntax.

I can do this:

const fs2: Pick<typeof fs, 'accessSync'> = fs
AccessFileSync.fs = fs2

but that's just silly.

The upside of this is that it is more precise and track closely to what you actually use.

The downside of this is that it is a bit more tedious. I wish control flow analysis can do that automatically for me in the future. :)

unional
  • 14,651
  • 5
  • 32
  • 56
  • by the way, typically in JavaScript, the convention for functions are camelCase. :) – unional Oct 09 '18 at 16:39
  • And since you are not using jest mock, you don't neeed `expect(AccessFileSync.fs.accessSync).toHaveBeenCalled();` – unional Oct 09 '18 at 16:40
  • We use the different case to differentiate our code from standard library calls, so when looking at functions in the source, we know they are from our library, not Javascript. – Steven Scott Oct 09 '18 at 16:49
  • Thanks. I saw that after the fact. Is there a way to fix the tslint error though, to remove the as any in the original declaration? – Steven Scott Oct 09 '18 at 16:56
  • I updated my code, but my test is still not catching the false. – Steven Scott Oct 09 '18 at 16:59
  • 1
    Likely because your mock is not throwing. I have updated the answer about linting. See if that helps. – unional Oct 09 '18 at 17:14
  • I had to do the second option, as the first simply gave a prefer-type-cast lint error, but the fs2 line did solve that. Also, throwing the error corrected my tests as well. Thanks alot. – Steven Scott Oct 09 '18 at 17:35
  • The `prefer-type-cast` error you get is a bug, you can open an issue to `tslint`. Be aware that linting is a tool to help the team to maintain a consistent style, which for the purpose of creating a more maintainable code. Don't let linting to over-restrict yourselves and hinder your expressiveness and flexibility. – unional Oct 09 '18 at 17:57