-
Notifications
You must be signed in to change notification settings - Fork 453
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
Hoist mock calls #239
Hoist mock calls #239
Changes from 11 commits
544a0a9
5de92d0
666c90e
b79a491
b26a949
b1a2224
87630d8
580f841
1e2a83a
d7e53a8
f8822db
609b533
7e927e3
4179926
f251ca9
31c9014
45da28a
467a853
541f407
73ad65b
03c841e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,7 @@ | ||
[*.js] | ||
[*.ts] | ||
indent_style = space | ||
indent_size = 4 | ||
indent_size = 4 | ||
|
||
[*.tsx] | ||
indent_style = space | ||
indent_size = 4 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,55 +3,57 @@ | |
* https://github.com/facebook/jest/blob/9b157c3a7c325c3971b2aabbe4c8ab4ce0b0c56d/packages/babel-jest/src/index.js | ||
*/ | ||
import * as jestPreset from 'babel-preset-jest'; | ||
import { JestConfig, PostProcessHook, PostProcessorOptions, TransformOptions } from './jest-types'; | ||
import {JestConfig, PostProcessHook, PostProcessorOptions, TransformOptions} from './jest-types'; | ||
import * as babel from 'babel-core'; | ||
import { CompilerOptions } from 'typescript/lib/typescript'; | ||
import {CompilerOptions} from 'typescript/lib/typescript'; | ||
|
||
function createBabelTransformer(options: PostProcessorOptions) { | ||
options = { | ||
...options, | ||
plugins: (options && options.plugins) || [], | ||
presets: ((options && options.presets) || []).concat([jestPreset]), | ||
retainLines: true, | ||
}; | ||
delete options.cacheDirectory; | ||
delete options.filename; | ||
options = { | ||
...options, | ||
plugins: (options && options.plugins) || [], | ||
presets: ((options && options.presets) || []).concat([jestPreset]), | ||
retainLines: true, | ||
}; | ||
delete options.cacheDirectory; | ||
delete options.filename; | ||
|
||
return (src: string, | ||
filename: string, | ||
config: JestConfig, | ||
transformOptions: TransformOptions): string => { | ||
const theseOptions = Object.assign({ filename }, options); | ||
if (transformOptions && transformOptions.instrument) { | ||
theseOptions.auxiliaryCommentBefore = ' istanbul ignore next '; | ||
// Copied from jest-runtime transform.js | ||
theseOptions.plugins = theseOptions.plugins.concat([ | ||
[ | ||
require('babel-plugin-istanbul').default, | ||
{ | ||
// files outside `cwd` will not be instrumented | ||
cwd: config.rootDir, | ||
exclude: [], | ||
}, | ||
], | ||
]); | ||
} | ||
return babel.transform(src, theseOptions).code; | ||
}; | ||
return (src: string, | ||
filename: string, | ||
config: JestConfig, | ||
transformOptions: TransformOptions): string => { | ||
const theseOptions = Object.assign({filename}, options); | ||
if (transformOptions && transformOptions.instrument) { | ||
theseOptions.auxiliaryCommentBefore = ' istanbul ignore next '; | ||
// Copied from jest-runtime transform.js | ||
theseOptions.plugins = theseOptions.plugins.concat([ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to use istanbul? With mapCoverage set to true, won't Jest take care of the instrumentation? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't actually know - I just assumed it was there for a reason in babel-jest. I'll try removing it and see what happens. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removing istanbul leads to some markedly different coverage numbers - I think it has to stay. |
||
[ | ||
require('babel-plugin-istanbul').default, | ||
{ | ||
// files outside `cwd` will not be instrumented | ||
cwd: config.rootDir, | ||
exclude: [], | ||
}, | ||
], | ||
]); | ||
} | ||
return babel.transform(src, theseOptions).code; | ||
}; | ||
} | ||
|
||
export const getPostProcessHook = (tsCompilerOptions: CompilerOptions, jestConfig: JestConfig, tsJestConfig: any): PostProcessHook => { | ||
if (tsJestConfig.skipBabel) { | ||
return (src) => src; //Identity function | ||
} | ||
if (tsJestConfig.skipBabel) { | ||
return (src) => src; //Identity function | ||
} | ||
|
||
//If we're not skipping babel | ||
if (tsCompilerOptions.allowSyntheticDefaultImports) { | ||
const plugins = ['transform-es2015-modules-commonjs']; | ||
return createBabelTransformer({ | ||
presets: [], | ||
plugins: plugins, | ||
}); | ||
} | ||
return (src) => src; //Identity function | ||
const plugins = []; | ||
//If we're not skipping babel | ||
if (tsCompilerOptions.allowSyntheticDefaultImports) { | ||
plugins.push('transform-es2015-modules-commonjs'); | ||
} | ||
|
||
|
||
return createBabelTransformer({ | ||
presets: [], | ||
plugins: plugins, | ||
}); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
import runJest from '../__helpers__/runJest'; | ||
|
||
describe('Jest.mock() calls', () => { | ||
|
||
it('Should run all tests using jest.mock() underneath the imports succesfully.', () => { | ||
const result = runJest('../hoist-test', ['--no-cache']); | ||
const output = result.output.toString(); | ||
|
||
expect(output).toContain('4 passed, 4 total'); | ||
expect(result.status).toBe(0); | ||
}); | ||
|
||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
import runJest from '../__helpers__/runJest'; | ||
|
||
describe('hello_world', () => { | ||
describe('Typescript errors', () => { | ||
|
||
it('should show the correct error locations in the typescript files', () => { | ||
|
||
|
@@ -9,9 +9,20 @@ describe('hello_world', () => { | |
const stderr = result.stderr.toString(); | ||
|
||
expect(result.status).toBe(1); | ||
expect(stderr).toContain('Hello.ts:13:11'); | ||
expect(stderr).toContain('Hello.ts:18:11'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't look right. If the source file hasn't changed, why should the line number change in the test? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch - the file has changed though. I wanted to be completely sure I didn't break anything, so I added a typescript interface to simple/Hello.ts - I wanted to put something in there that'd fill up a few lines, but I knew would be stripped by tsc There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This uses simple/Hello.ts as the source, right? That should show an error on line 13 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. Not since I edited the file ;) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I checked the wrong version of the file while making that comment. It looks alright! |
||
expect(stderr).toContain('Hello.test.ts:9:19'); | ||
|
||
}); | ||
|
||
it('Should show the correct error locations in async typescript files', async () => { | ||
const result = runJest('../simple-async', ['--no-cache']); | ||
|
||
const stderr = result.stderr.toString(); | ||
|
||
expect(result.status).toBe(1); | ||
expect(stderr).toContain('Hello.ts:13:11'); | ||
expect(stderr).toContain('Hello.test.ts:9:21'); | ||
}); | ||
|
||
|
||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
|
||
import * as path from 'path'; | ||
import {SomeClass, SomeFunctionDeclaredAsConst, SomeFunction} from '../src/things-to-mock'; | ||
|
||
jest.mock('../src/things-to-mock'); | ||
jest.mock('path'); | ||
|
||
|
||
describe('Global mocks', () => { | ||
it('A global var should be mocked when the jest.mock call is underneath', () => { | ||
expect((path.basename as any).mock).toBeDefined(); | ||
}); | ||
}); | ||
|
||
describe('Local mocks', () => { | ||
it('Jest should be able to mock a local class', () => { | ||
expect((SomeClass as any).mockReturnValue).toBeDefined(); | ||
}); | ||
|
||
it('Jest should be able to mock a local class', () => { | ||
expect((SomeFunction as any).mockReturnValueOnce).toBeDefined(); | ||
}); | ||
|
||
it('Jest should be able to mock a local class', () => { | ||
expect((SomeFunctionDeclaredAsConst as any).mockImplementation).toBeDefined(); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
{ | ||
"jest": { | ||
"transform": { | ||
".(ts|tsx)": "../../preprocessor.js" | ||
}, | ||
"moduleDirectories": ["node_modules", "src"], | ||
"testRegex": "(/__tests__/.*|\\.(test|spec))\\.(ts|tsx|js)$", | ||
"moduleFileExtensions": [ | ||
"ts", | ||
"tsx", | ||
"js" | ||
] | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
|
||
|
||
export class SomeClass { | ||
} | ||
|
||
export const SomeFunctionDeclaredAsConst = () => 'originalValue'; | ||
|
||
export function SomeFunction(){ | ||
return 'original'; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
{ | ||
"compilerOptions": { | ||
"target": "ES5", | ||
"module": "commonjs", | ||
"moduleResolution": "node", | ||
"noEmitOnError": false, | ||
"jsx": "react", | ||
"allowJs": true, | ||
"baseUrl": "src", | ||
"allowSyntheticDefaultImports": false | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't really need retainLines, do we?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unsure. It breaks a bunch of tests if it is not included. Perhaps it only needs to be set if inline sourcemaps are turned off?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to set inlineSourcemaps to true for sourcemaps to work. This is because we don't generate sourcemap files on disk during testing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean "sourceMaps": "inline" in babel or in tsconfig? I think we already set it for tsconfig.
I have tried setting it for babel but it gives me off by one error for every tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inlineSourcemaps: true
for typescript anddevtool: inline-source-map
for babelThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't devtool: inline-source-map a webpack thing? I'm not seeing in in the babel docs anywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right.
I'm gonna lay off of this thing for today. I don't think I'm helping
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that babel does have an inputSourceMap though. What problem are we trying to solve exactly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's cool. I think this is worth exploring, but if we think the sourcemaps work as-is, I don't think there's a need to change the code we copied from babel-jest