Reputation: 11250
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.
Upvotes: 0
Views: 346
Reputation: 15589
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. :)
Upvotes: 1