Commit bd3e719e authored by Andrew Fontaine's avatar Andrew Fontaine

Merge branch 'fe-remove-manual-node-mocks' into 'master'

Remove manual mocking of node

See merge request gitlab-org/gitlab!27861
parents a4b03f35 d945d091
...@@ -547,16 +547,26 @@ The more challenging part are mocks, which can be used for functions or even dep ...@@ -547,16 +547,26 @@ The more challenging part are mocks, which can be used for functions or even dep
### Manual module mocks ### Manual module mocks
Jest supports [manual module mocks](https://jestjs.io/docs/en/manual-mocks) by placing a mock in a `__mocks__/` directory next to the source module. **Don't do this.** We want to keep all of our test-related code in one place (the `spec/` folder), and the logic that Jest uses to apply mocks from `__mocks__/` is rather inconsistent. Manual mocks are used to mock modules across the entire Jest environment. This is a very powerful testing tool that helps simplify
unit testing by mocking out modules which cannot be easily consumned in our test environment.
Instead, our test runner detects manual mocks from `spec/frontend/mocks/`. Any mock placed here is automatically picked up and injected whenever you import its source module. > NOTE: Do not use manual mocks if a mock should not be consistently applied (i.e. it's only needed by a few specs).
> Instead, consider using `jest.mock` in the relevant spec file.
#### Where should I put manual mocks?
Jest supports [manual module mocks](https://jestjs.io/docs/en/manual-mocks) by placing a mock in a `__mocks__/` directory next to the source module
(e.g. `app/assets/javascripts/ide/__mocks__`). **Don't do this.** We want to keep all of our test-related code in one place (the `spec/` folder).
If a manual mock is needed for a `node_modules` package, please use the `spec/frontend/__mocks__` folder. Here's an example of
a [Jest mock for the package `monaco-editor`](https://gitlab.com/gitlab-org/gitlab/blob/b7f914cddec9fc5971238cdf12766e79fa1629d7/spec/frontend/__mocks__/monaco-editor/index.js#L1).
If a manual mock is needed for a CE module, please place it in `spec/frontend/mocks/ce`.
- Files in `spec/frontend/mocks/ce` will mock the corresponding CE module from `app/assets/javascripts`, mirroring the source module's path. - Files in `spec/frontend/mocks/ce` will mock the corresponding CE module from `app/assets/javascripts`, mirroring the source module's path.
- Example: `spec/frontend/mocks/ce/lib/utils/axios_utils` will mock the module `~/lib/utils/axios_utils`. - Example: `spec/frontend/mocks/ce/lib/utils/axios_utils` will mock the module `~/lib/utils/axios_utils`.
- Files in `spec/frontend/mocks/node` will mock NPM packages of the same name or path.
- We don't support mocking EE modules yet. - We don't support mocking EE modules yet.
- If a mock is found for which a source module doesn't exist, the test suite will fail. 'Virtual' mocks, or mocks that don't have a 1-to-1 association with a source module, are not supported yet.
If a mock is found for which a source module doesn't exist, the test suite will fail. 'Virtual' mocks, or mocks that don't have a 1-to-1 association with a source module, are not supported yet.
### Writing a mock ### Writing a mock
......
...@@ -16,7 +16,6 @@ const prefixMap = [ ...@@ -16,7 +16,6 @@ const prefixMap = [
// E.g. the mock ce/foo/bar maps to require path ~/foo/bar // E.g. the mock ce/foo/bar maps to require path ~/foo/bar
{ mocksRoot: 'ce', requirePrefix: '~' }, { mocksRoot: 'ce', requirePrefix: '~' },
// { mocksRoot: 'ee', requirePrefix: 'ee' }, // We'll deal with EE-specific mocks later // { mocksRoot: 'ee', requirePrefix: 'ee' }, // We'll deal with EE-specific mocks later
{ mocksRoot: 'node', requirePrefix: '' },
// { mocksRoot: 'virtual', requirePrefix: '' }, // We'll deal with virtual mocks later // { mocksRoot: 'virtual', requirePrefix: '' }, // We'll deal with virtual mocks later
]; ];
......
...@@ -34,9 +34,8 @@ describe('mocks_helper.js', () => { ...@@ -34,9 +34,8 @@ describe('mocks_helper.js', () => {
it('enumerates through mock file roots', () => { it('enumerates through mock file roots', () => {
setupManualMocks(); setupManualMocks();
expect(fs.existsSync).toHaveBeenCalledTimes(2); expect(fs.existsSync).toHaveBeenCalledTimes(1);
expect(fs.existsSync).toHaveBeenNthCalledWith(1, absPath('ce')); expect(fs.existsSync).toHaveBeenNthCalledWith(1, absPath('ce'));
expect(fs.existsSync).toHaveBeenNthCalledWith(2, absPath('node'));
expect(readdir.sync).toHaveBeenCalledTimes(0); expect(readdir.sync).toHaveBeenCalledTimes(0);
}); });
...@@ -66,19 +65,6 @@ describe('mocks_helper.js', () => { ...@@ -66,19 +65,6 @@ describe('mocks_helper.js', () => {
expect(setMock).toHaveBeenNthCalledWith(2, '~/lib/utils/util', './ce/lib/utils/util'); expect(setMock).toHaveBeenNthCalledWith(2, '~/lib/utils/util', './ce/lib/utils/util');
}); });
it('sets up mocks for node_modules', () => {
fs.existsSync.mockImplementation(root => root.endsWith('node'));
readdir.sync.mockReturnValue(['jquery', '@babel/core']);
setupManualMocks();
expect(readdir.sync).toHaveBeenCalledTimes(1);
expect(readdir.sync.mock.calls[0][0]).toBe(absPath('node'));
expect(setMock).toHaveBeenCalledTimes(2);
expect(setMock).toHaveBeenNthCalledWith(1, 'jquery', './node/jquery');
expect(setMock).toHaveBeenNthCalledWith(2, '@babel/core', './node/@babel/core');
});
it('sets up mocks for all roots', () => { it('sets up mocks for all roots', () => {
const files = { const files = {
[absPath('ce')]: ['root', 'lib/utils/util'], [absPath('ce')]: ['root', 'lib/utils/util'],
...@@ -89,15 +75,12 @@ describe('mocks_helper.js', () => { ...@@ -89,15 +75,12 @@ describe('mocks_helper.js', () => {
readdir.sync.mockImplementation(root => files[root]); readdir.sync.mockImplementation(root => files[root]);
setupManualMocks(); setupManualMocks();
expect(readdir.sync).toHaveBeenCalledTimes(2); expect(readdir.sync).toHaveBeenCalledTimes(1);
expect(readdir.sync.mock.calls[0][0]).toBe(absPath('ce')); expect(readdir.sync.mock.calls[0][0]).toBe(absPath('ce'));
expect(readdir.sync.mock.calls[1][0]).toBe(absPath('node'));
expect(setMock).toHaveBeenCalledTimes(4); expect(setMock).toHaveBeenCalledTimes(2);
expect(setMock).toHaveBeenNthCalledWith(1, '~/root', './ce/root'); expect(setMock).toHaveBeenNthCalledWith(1, '~/root', './ce/root');
expect(setMock).toHaveBeenNthCalledWith(2, '~/lib/utils/util', './ce/lib/utils/util'); expect(setMock).toHaveBeenNthCalledWith(2, '~/lib/utils/util', './ce/lib/utils/util');
expect(setMock).toHaveBeenNthCalledWith(3, 'jquery', './node/jquery');
expect(setMock).toHaveBeenNthCalledWith(4, '@babel/core', './node/@babel/core');
}); });
it('fails when given a virtual mock', () => { it('fails when given a virtual mock', () => {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment