From 6cdff6872e5e773cee324cd05767ddf67f841e86 Mon Sep 17 00:00:00 2001 From: Cameron Little Date: Sun, 16 Aug 2020 11:09:45 +0200 Subject: [PATCH 1/9] Add failing tests --- e2e/__tests__/locationInResults.test.ts | 45 ++++++++++++++++------- e2e/location-in-results/__tests__/test.js | 8 ++++ 2 files changed, 40 insertions(+), 13 deletions(-) diff --git a/e2e/__tests__/locationInResults.test.ts b/e2e/__tests__/locationInResults.test.ts index e444ed7b6830..39e2b0dfbe36 100644 --- a/e2e/__tests__/locationInResults.test.ts +++ b/e2e/__tests__/locationInResults.test.ts @@ -13,13 +13,11 @@ it('defaults to null for location', () => { const assertions = result.testResults[0].assertionResults; expect(result.success).toBe(true); - expect(result.numTotalTests).toBe(6); - expect(assertions[0].location).toBeNull(); - expect(assertions[1].location).toBeNull(); - expect(assertions[2].location).toBeNull(); - expect(assertions[3].location).toBeNull(); - expect(assertions[4].location).toBeNull(); - expect(assertions[5].location).toBeNull(); + expect(result.numTotalTests).toBe(10); + expect(assertions).toHaveLength(10); + for (const assertion of assertions) { + expect(assertion.location).toBeNull(); + } }); it('adds correct location info when provided with flag', () => { @@ -29,7 +27,8 @@ it('adds correct location info when provided with flag', () => { const assertions = result.testResults[0].assertionResults; expect(result.success).toBe(true); - expect(result.numTotalTests).toBe(6); + expect(result.numTotalTests).toBe(10); + expect(assertions[0].location).toEqual({ column: 1, line: 12, @@ -45,20 +44,40 @@ it('adds correct location info when provided with flag', () => { line: 20, }); - // Technically the column should be 3, but callsites is not correct. - // jest-circus uses stack-utils + asyncErrors which resolves this. expect(assertions[3].location).toEqual({ - column: isJestCircusRun() ? 3 : 2, - line: 25, + column: 22, + line: 24, }); expect(assertions[4].location).toEqual({ + column: 22, + line: 24, + }); + + // Technically the column should be 3, but callsites is not correct. + // jest-circus uses stack-utils + asyncErrors which resolves this. + expect(assertions[5].location).toEqual({ column: isJestCircusRun() ? 3 : 2, line: 29, }); - expect(assertions[5].location).toEqual({ + expect(assertions[6].location).toEqual({ column: isJestCircusRun() ? 3 : 2, line: 33, }); + + expect(assertions[7].location).toEqual({ + column: isJestCircusRun() ? 3 : 2, + line: 37, + }); + + expect(assertions[8].location).toEqual({ + column: isJestCircusRun() ? 25 : 24, + line: 41, + }); + + expect(assertions[9].location).toEqual({ + column: isJestCircusRun() ? 25 : 24, + line: 41, + }); }); diff --git a/e2e/location-in-results/__tests__/test.js b/e2e/location-in-results/__tests__/test.js index dfc62f3a91fd..fa9c66a0a287 100644 --- a/e2e/location-in-results/__tests__/test.js +++ b/e2e/location-in-results/__tests__/test.js @@ -21,6 +21,10 @@ fit('fit no ancestors', () => { expect(true).toBeTruthy(); }); +it.each([true, true])('it each no ancestors', () => { + expect(true).toBeTruthy(); +}); + describe('nested', () => { it('it nested', () => { expect(true).toBeTruthy(); @@ -33,4 +37,8 @@ describe('nested', () => { fit('fit nested', () => { expect(true).toBeTruthy(); }); + + it.each([true, true])('it each nested', () => { + expect(true).toBeTruthy(); + }); }); From 85fe9425205b08033bb9343a28804c9a75d9ecc0 Mon Sep 17 00:00:00 2001 From: Cameron Little Date: Sun, 16 Aug 2020 12:16:05 +0200 Subject: [PATCH 2/9] Deduplicate Jasmine global test location wrappers --- packages/jest-jasmine2/src/index.ts | 50 +++++++++++------------------ 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/packages/jest-jasmine2/src/index.ts b/packages/jest-jasmine2/src/index.ts index 7c081b7846b1..fcb593f40609 100644 --- a/packages/jest-jasmine2/src/index.ts +++ b/packages/jest-jasmine2/src/index.ts @@ -47,38 +47,26 @@ async function jasmine2( // TODO: Remove config option if V8 exposes some way of getting location of caller // in a future version if (config.testLocationInResults === true) { - const originalIt = environment.global.it; - environment.global.it = ((...args) => { - const stack = getCallsite(1, runtime.getSourceMaps()); - const it = originalIt(...args); - - // @ts-expect-error - it.result.__callsite = stack; - - return it; - }) as Global.Global['it']; - - const originalXit = environment.global.xit; - environment.global.xit = ((...args) => { - const stack = getCallsite(1, runtime.getSourceMaps()); - const xit = originalXit(...args); - - // @ts-expect-error - xit.result.__callsite = stack; - - return xit; - }) as Global.Global['xit']; - - const originalFit = environment.global.fit; - environment.global.fit = ((...args) => { - const stack = getCallsite(1, runtime.getSourceMaps()); - const fit = originalFit(...args); - - // @ts-expect-error - fit.result.__callsite = stack; + function wrapIt(original: T): T { + const wrapped = ( + testName: Global.TestName, + fn: Global.TestFn, + timeout?: number, + ) => { + const stack = getCallsite(1, runtime.getSourceMaps()); + const it = original(testName, fn, timeout); + + // @ts-expect-error + it.result.__callsite = stack; + + return it; + }; + return (wrapped as any) as T; + } - return fit; - }) as Global.Global['fit']; + environment.global.it = wrapIt(environment.global.it); + environment.global.xit = wrapIt(environment.global.xit); + environment.global.fit = wrapIt(environment.global.fit); } jasmineAsyncInstall(globalConfig, environment.global); From 1f6138f68cd6d7f4c80e99f36181c07e18d1e95a Mon Sep 17 00:00:00 2001 From: Cameron Little Date: Sun, 16 Aug 2020 12:16:58 +0200 Subject: [PATCH 3/9] Add hacky fix for bad each location in Jasmine --- packages/jest-jasmine2/src/index.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/jest-jasmine2/src/index.ts b/packages/jest-jasmine2/src/index.ts index fcb593f40609..6c8adb3cf85e 100644 --- a/packages/jest-jasmine2/src/index.ts +++ b/packages/jest-jasmine2/src/index.ts @@ -53,9 +53,12 @@ async function jasmine2( fn: Global.TestFn, timeout?: number, ) => { - const stack = getCallsite(1, runtime.getSourceMaps()); + let stack = getCallsite(1, runtime.getSourceMaps()); const it = original(testName, fn, timeout); + if (stack.getFileName()?.includes('/jest-each/')) { + stack = getCallsite(4, runtime.getSourceMaps()); + } // @ts-expect-error it.result.__callsite = stack; From af9414ac799e99d26489545c599b9f959c9c910d Mon Sep 17 00:00:00 2001 From: Cameron Little Date: Sun, 16 Aug 2020 12:23:59 +0200 Subject: [PATCH 4/9] Hacky fix for each test location with jest-circus --- e2e/__tests__/locationInResults.test.ts | 8 ++++---- packages/jest-circus/src/utils.ts | 6 +++++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/e2e/__tests__/locationInResults.test.ts b/e2e/__tests__/locationInResults.test.ts index 39e2b0dfbe36..25808059c29c 100644 --- a/e2e/__tests__/locationInResults.test.ts +++ b/e2e/__tests__/locationInResults.test.ts @@ -45,12 +45,12 @@ it('adds correct location info when provided with flag', () => { }); expect(assertions[3].location).toEqual({ - column: 22, + column: isJestCircusRun() ? 1 : 22, line: 24, }); expect(assertions[4].location).toEqual({ - column: 22, + column: isJestCircusRun() ? 1 : 22, line: 24, }); @@ -72,12 +72,12 @@ it('adds correct location info when provided with flag', () => { }); expect(assertions[8].location).toEqual({ - column: isJestCircusRun() ? 25 : 24, + column: isJestCircusRun() ? 3 : 24, line: 41, }); expect(assertions[9].location).toEqual({ - column: isJestCircusRun() ? 25 : 24, + column: isJestCircusRun() ? 3 : 24, line: 41, }); }); diff --git a/packages/jest-circus/src/utils.ts b/packages/jest-circus/src/utils.ts index 82b4b78e52d4..ea32144d32be 100644 --- a/packages/jest-circus/src/utils.ts +++ b/packages/jest-circus/src/utils.ts @@ -300,7 +300,11 @@ export const makeSingleTestResult = ( let location = null; if (includeTestLocationInResult) { const stackLine = test.asyncError.stack.split('\n')[1]; - const parsedLine = stackUtils.parseLine(stackLine); + let parsedLine = stackUtils.parseLine(stackLine); + if (parsedLine?.file?.includes('/jest-each/')) { + const stackLine = test.asyncError.stack.split('\n')[4]; + parsedLine = stackUtils.parseLine(stackLine); + } if ( parsedLine && typeof parsedLine.column === 'number' && From 5d5d39e5418c27e993568369ddf0ee194429ded1 Mon Sep 17 00:00:00 2001 From: Cameron Little Date: Sun, 16 Aug 2020 14:07:40 +0200 Subject: [PATCH 5/9] Fix windows support with system path separators --- packages/jest-circus/src/utils.ts | 3 ++- packages/jest-jasmine2/src/index.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/jest-circus/src/utils.ts b/packages/jest-circus/src/utils.ts index ea32144d32be..1f0457cf0dc3 100644 --- a/packages/jest-circus/src/utils.ts +++ b/packages/jest-circus/src/utils.ts @@ -5,6 +5,7 @@ * LICENSE file in the root directory of this source tree. */ +import * as path from 'path'; import type {Circus} from '@jest/types'; import {convertDescriptorToString, formatTime} from 'jest-util'; import isGeneratorFn from 'is-generator-fn'; @@ -301,7 +302,7 @@ export const makeSingleTestResult = ( if (includeTestLocationInResult) { const stackLine = test.asyncError.stack.split('\n')[1]; let parsedLine = stackUtils.parseLine(stackLine); - if (parsedLine?.file?.includes('/jest-each/')) { + if (parsedLine?.file?.includes(`${path.sep}jest-each${path.sep}`)) { const stackLine = test.asyncError.stack.split('\n')[4]; parsedLine = stackUtils.parseLine(stackLine); } diff --git a/packages/jest-jasmine2/src/index.ts b/packages/jest-jasmine2/src/index.ts index 6c8adb3cf85e..3408ab29036c 100644 --- a/packages/jest-jasmine2/src/index.ts +++ b/packages/jest-jasmine2/src/index.ts @@ -56,7 +56,7 @@ async function jasmine2( let stack = getCallsite(1, runtime.getSourceMaps()); const it = original(testName, fn, timeout); - if (stack.getFileName()?.includes('/jest-each/')) { + if (stack.getFileName()?.includes(`${path.sep}jest-each${path.sep}`)) { stack = getCallsite(4, runtime.getSourceMaps()); } // @ts-expect-error From 2a0ec8c422dae9ee38b1f1ccc873cc52224487c8 Mon Sep 17 00:00:00 2001 From: Cameron Little Date: Sun, 16 Aug 2020 16:21:17 +0200 Subject: [PATCH 6/9] Don't grab lines twice --- packages/jest-circus/src/utils.ts | 5 +++-- packages/jest-jasmine2/src/index.ts | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/jest-circus/src/utils.ts b/packages/jest-circus/src/utils.ts index 1f0457cf0dc3..a82aa6e7a675 100644 --- a/packages/jest-circus/src/utils.ts +++ b/packages/jest-circus/src/utils.ts @@ -300,10 +300,11 @@ export const makeSingleTestResult = ( let location = null; if (includeTestLocationInResult) { - const stackLine = test.asyncError.stack.split('\n')[1]; + const stackLines = test.asyncError.stack.split('\n'); + const stackLine = stackLines[1]; let parsedLine = stackUtils.parseLine(stackLine); if (parsedLine?.file?.includes(`${path.sep}jest-each${path.sep}`)) { - const stackLine = test.asyncError.stack.split('\n')[4]; + const stackLine = stackLines[4]; parsedLine = stackUtils.parseLine(stackLine); } if ( diff --git a/packages/jest-jasmine2/src/index.ts b/packages/jest-jasmine2/src/index.ts index 3408ab29036c..ea2af250da6f 100644 --- a/packages/jest-jasmine2/src/index.ts +++ b/packages/jest-jasmine2/src/index.ts @@ -53,11 +53,12 @@ async function jasmine2( fn: Global.TestFn, timeout?: number, ) => { - let stack = getCallsite(1, runtime.getSourceMaps()); + const sourcemaps = runtime.getSourceMaps(); + let stack = getCallsite(1, sourcemaps); const it = original(testName, fn, timeout); if (stack.getFileName()?.includes(`${path.sep}jest-each${path.sep}`)) { - stack = getCallsite(4, runtime.getSourceMaps()); + stack = getCallsite(4, sourcemaps); } // @ts-expect-error it.result.__callsite = stack; From 82ebb50a954dfe2a561d9e9a8117e98abfa110bf Mon Sep 17 00:00:00 2001 From: Cameron Little Date: Mon, 17 Aug 2020 08:06:48 +0200 Subject: [PATCH 7/9] Code review: Better method for determining each call --- packages/jest-circus/src/utils.ts | 4 +++- packages/jest-jasmine2/src/index.ts | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/jest-circus/src/utils.ts b/packages/jest-circus/src/utils.ts index a82aa6e7a675..a133e993ad10 100644 --- a/packages/jest-circus/src/utils.ts +++ b/packages/jest-circus/src/utils.ts @@ -18,6 +18,8 @@ import {ROOT_DESCRIBE_BLOCK_NAME, getState} from './state'; const stackUtils = new StackUtils({cwd: 'A path that does not exist'}); +const jestEachBuildDir = path.dirname(require.resolve('jest-each')); + export const makeDescribe = ( name: Circus.BlockName, parent?: Circus.DescribeBlock, @@ -303,7 +305,7 @@ export const makeSingleTestResult = ( const stackLines = test.asyncError.stack.split('\n'); const stackLine = stackLines[1]; let parsedLine = stackUtils.parseLine(stackLine); - if (parsedLine?.file?.includes(`${path.sep}jest-each${path.sep}`)) { + if (parsedLine?.file?.startsWith(jestEachBuildDir)) { const stackLine = stackLines[4]; parsedLine = stackUtils.parseLine(stackLine); } diff --git a/packages/jest-jasmine2/src/index.ts b/packages/jest-jasmine2/src/index.ts index ea2af250da6f..953a2df10666 100644 --- a/packages/jest-jasmine2/src/index.ts +++ b/packages/jest-jasmine2/src/index.ts @@ -22,6 +22,8 @@ import type {Jasmine as JestJasmine} from './types'; const JASMINE = require.resolve('./jasmine/jasmineLight'); +const jestEachBuildDir = path.dirname(require.resolve('jest-each')); + async function jasmine2( globalConfig: Config.GlobalConfig, config: Config.ProjectConfig, @@ -57,7 +59,7 @@ async function jasmine2( let stack = getCallsite(1, sourcemaps); const it = original(testName, fn, timeout); - if (stack.getFileName()?.includes(`${path.sep}jest-each${path.sep}`)) { + if (stack.getFileName()?.startsWith(jestEachBuildDir)) { stack = getCallsite(4, sourcemaps); } // @ts-expect-error From 3dd34a2fa7d6fef411a70c9592c67d115e07c3fc Mon Sep 17 00:00:00 2001 From: Cameron Little Date: Mon, 17 Aug 2020 08:06:54 +0200 Subject: [PATCH 8/9] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index dd0ebbcf853a..3bb378f4f1c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - `[jest-reporters]` Fixes notify reporter on Linux (using notify-send) ([#10393](https://github.com/facebook/jest/pull/10400)) - `[jest-snapshot]` Correctly handles arrays and property matchers in snapshots ([#10404](https://github.com/facebook/jest/pull/10404)) +- `[jest-jasmine2, jest-circus]` Find correct location for `test.each` tests ([#10413](https://github.com/facebook/jest/pull/10413)) ### Chore & Maintenance From af6565bc03f70bb461476d6d3da1b8572c12751b Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Mon, 17 Aug 2020 15:55:05 +0200 Subject: [PATCH 9/9] sort changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bb378f4f1c0..9c14ce1b20da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,9 @@ ### Fixes +- `[jest-circus, jest-jasmine2]` Find correct location for `test.each` tests ([#10413](https://github.com/facebook/jest/pull/10413)) - `[jest-reporters]` Fixes notify reporter on Linux (using notify-send) ([#10393](https://github.com/facebook/jest/pull/10400)) - `[jest-snapshot]` Correctly handles arrays and property matchers in snapshots ([#10404](https://github.com/facebook/jest/pull/10404)) -- `[jest-jasmine2, jest-circus]` Find correct location for `test.each` tests ([#10413](https://github.com/facebook/jest/pull/10413)) ### Chore & Maintenance