Merge pull request jest-community#632 from connectdotz/fix-debug-test…

…-name

Use fullName in TestResult

CHANGELOG.md

@@ -5,7 +5,7 @@ Bug-fixes within the same version aren't needed
 
 ## Master
 
-
+* use assertion's fullName in TestResult instead of the source test name - @connectdotz
 -->
 
 ### 4.0.0-alpha.1

package.json

@@ -289,7 +289,7 @@
   "devDependencies": {
     "@types/istanbul-lib-coverage": "^2.0.2",
     "@types/istanbul-lib-source-maps": "^4.0.1",
-    "@types/jest": "^25.2.1",
+    "@types/jest": "^26.0.15",
     "@types/node": "^8.0.31",
     "@types/vscode": "^1.23.0",
     "@typescript-eslint/eslint-plugin": "^2.32.0",
@@ -304,11 +304,11 @@
     "eslint-plugin-jsdoc": "^25.2.0",
     "eslint-plugin-prefer-arrow": "^1.2.1",
     "eslint-plugin-prettier": "^3.1.3",
-    "jest": "^25.5.0",
+    "jest": "^26.6.1",
     "prettier": "^2.0.5",
     "raw-loader": "^4.0.1",
     "rimraf": "^3.0.2",
-    "ts-jest": "^25.4.0",
+    "ts-jest": "^26.4.2",
     "ts-loader": "^7.0.1",
     "typescript": "^3.8.3",
     "vscode-test": "^1.3.0",

src/TestResults/TestResult.ts

@@ -20,6 +20,12 @@ export interface LocationRange {
 export interface TestResult extends LocationRange {
   name: string;
 
+  names: {
+    src: string;
+    assertionTitle?: string;
+    assertionFullName?: string;
+  };
+
   status: TestReconciliationState;
   shortMessage?: string;
   terseMessage?: string;

src/TestResults/match-by-context.ts

@@ -73,7 +73,12 @@ export const toMatchResult = (
 
   // Note the shift from one-based to zero-based line number and columns
   return {
-    name: test.name,
+    name: assertion?.fullName ?? assertion?.title ?? test.name,
+    names: {
+      src: test.name,
+      assertionTitle: assertion?.title,
+      assertionFullName: assertion?.fullName,
+    },
     start: adjustLocation(test.start),
     end: adjustLocation(test.end),
     status: assertion?.status ?? TestReconciliationState.Unknown,

tests/TestResults/TestResult.test.ts

@@ -1,6 +1,6 @@
 jest.unmock('../../src/TestResults/TestResult');
 jest.unmock('../../src/helpers');
-jest.mock('path', () => ({ sep: require.requireActual('path').sep }));
+jest.mock('path', () => ({ sep: jest.requireActual('path').sep }));
 
 import * as TestResult from '../../src/TestResults/TestResult';
 import * as path from 'path';

tests/TestResults/TestResultProvider.test.ts

@@ -209,7 +209,7 @@ describe('TestResultProvider', () => {
       ]);
 
       mockParseTest([testBlock, testBlock3, testBlock2]);
-      assertionsForTestFile.mockReturnValueOnce([
+      const assertions = [
         helper.makeAssertion(testBlock.name, TestReconciliationState.KnownSuccess, [], [1, 0]),
         helper.makeAssertion(
           'template literal I got something like this',
@@ -223,10 +223,20 @@ describe('TestResultProvider', () => {
           [],
           [3, 0]
         ),
-      ]);
+      ];
+      assertionsForTestFile.mockReturnValueOnce(assertions);
       const actual = sut.getResults(filePath);
       expect(actual).toHaveLength(3);
-      expect(actual.map((a) => a.name)).toEqual([testBlock.name, testBlock2.name, testBlock3.name]);
+      expect(actual.map((a) => a.name)).toEqual([
+        assertions[0].fullName,
+        assertions[1].fullName,
+        assertions[2].fullName,
+      ]);
+      expect(actual.map((a) => a.names.src)).toEqual([
+        testBlock.name,
+        testBlock2.name,
+        testBlock3.name,
+      ]);
       expect(actual.map((a) => a.status)).toEqual([
         TestReconciliationState.KnownSuccess,
         TestReconciliationState.KnownFail,

tests/TestResults/match-by-context.test.ts

@@ -134,6 +134,11 @@ describe('buildSourceContainer', () => {
   });
 });
 describe('matchTestAssertions', () => {
+  const mockWarn = jest.fn();
+  beforeEach(() => {
+    jest.resetAllMocks();
+    console.warn = mockWarn;
+  });
   it('tests are matched by context position regardless name and line', () => {
     const t1 = helper.makeItBlock('test-1', [1, 0, 5, 0]);
     const t2 = helper.makeItBlock('test-2-${num}', [6, 0, 7, 0]);
@@ -144,7 +149,10 @@ describe('matchTestAssertions', () => {
     const matched = match.matchTestAssertions('a file', sourceRoot, [a1, a2]);
 
     expect(matched).toHaveLength(2);
-    expect(matched.map((m) => m.name)).toEqual(['test-1', 'test-2-${num}']);
+    expect(matched.map((m) => m.name)).toEqual(['test-1', 'test-2-100']);
+    expect(matched.map((m) => m.names.src)).toEqual(['test-1', 'test-2-${num}']);
+    expect(matched.map((m) => m.names.assertionTitle)).toEqual(['test-1', 'test-2-100']);
+    expect(matched.map((m) => m.names.assertionFullName)).toEqual(['test-1', 'test-2-100']);
     expect(matched.map((m) => m.status)).toEqual(['KnownFail', 'KnownSuccess']);
   });
   it('can match tests with the same name but in different describe blocks', () => {
@@ -156,7 +164,10 @@ describe('matchTestAssertions', () => {
     const a1 = helper.makeAssertion('test-1', 'KnownFail', [], [0, 0]);
     const a2 = helper.makeAssertion('test-1', 'KnownSuccess', ['d-1'], [5, 0]);
     const matched = match.matchTestAssertions('a file', sourceRoot, [a1, a2]);
-    expect(matched.map((m) => m.name)).toEqual(['test-1', 'test-1']);
+    expect(matched.map((m) => m.name)).toEqual(['test-1', 'd-1 test-1']);
+    expect(matched.map((m) => m.names.src)).toEqual(['test-1', 'test-1']);
+    expect(matched.map((m) => m.names.assertionTitle)).toEqual(['test-1', 'test-1']);
+    expect(matched.map((m) => m.names.assertionFullName)).toEqual(['test-1', 'd-1 test-1']);
     expect(matched.map((m) => m.status)).toEqual(['KnownFail', 'KnownSuccess']);
     expect(matched.map((m) => m.start.line)).toEqual([0, 5]);
     expect(matched.map((m) => m.end.line)).toEqual([4, 6]);
@@ -201,9 +212,9 @@ describe('matchTestAssertions', () => {
       const matched = match.matchTestAssertions('a file', sourceRoot, [a1, a2, a4]);
       expect(matched.map((m) => [m.name, m.status])).toEqual([
         ['test-1', 'KnownSuccess'],
-        ['test-2', 'KnownFail'],
+        ['d-1 test-2', 'KnownFail'],
         ['test-3', 'Unknown'],
-        ['test-4', 'KnownSuccess'],
+        ['d-1 d-1-1 test-4', 'KnownSuccess'],
       ]);
     });
     it('describe block will fail if context mismatch and name lookup failed', () => {
@@ -267,6 +278,34 @@ describe('matchTestAssertions', () => {
       expect(matched[0].end).toEqual({ line: 19, column: 0 });
       expect(matched[0].lineNumberOfError).toEqual(12);
     });
+    describe('test result name', () => {
+      it('use the first failed assertion, if exist', () => {
+        const [root, assertions] = createTestData([
+          'KnownSuccess',
+          ['KnownFail', 13],
+          'KnownSuccess',
+          ['KnownFail', 20],
+        ]);
+        const matched = match.matchTestAssertions('a file', root, assertions);
+        expect(matched).toHaveLength(1);
+        expect(matched[0].name).toEqual('test-1');
+        expect(matched[0].status).toEqual('KnownFail');
+      });
+      describe('if no failed assertion, the first assertion will be used', () => {
+        // eat-our-own-dog-food: use jest .each so we can ensure it worked for our users
+        const entries = [
+          [['KnownSuccess', 'KnownSuccess'], 'KnownSuccess'],
+          [['Unknown', 'Unknown'], 'Unknown'],
+        ];
+        it.each(entries)('with test entry %#', (entry, status) => {
+          const [root, assertions] = createTestData(entry as TestReconciliationState[]);
+          const matched = match.matchTestAssertions('a file', root, assertions);
+          expect(matched).toHaveLength(1);
+          expect(matched[0].name).toEqual('test-0');
+          expect(matched[0].status).toEqual(status);
+        });
+      });
+    });
     it('test is succeeded if all assertions are successful', () => {
       const [root, assertions] = createTestData(['KnownSuccess', 'KnownSuccess', 'KnownSuccess']);
       const matched = match.matchTestAssertions('a file', root, assertions);
@@ -283,5 +322,49 @@ describe('matchTestAssertions', () => {
       expect(matched).toHaveLength(1);
       expect(matched[0].status).toEqual(TestReconciliationState.KnownSkip);
     });
+    it('test name', () => {
+      const [root, assertions] = createTestData([
+        TestReconciliationState.KnownSkip,
+        TestReconciliationState.KnownSkip,
+        TestReconciliationState.KnownSkip,
+      ]);
+      const matched = match.matchTestAssertions('a file', root, assertions);
+      expect(matched).toHaveLength(1);
+      expect(matched[0].status).toEqual(TestReconciliationState.KnownSkip);
+    });
+  });
+  it('test name precedence: assertion.fullName > assertion.title > testSource.name', () => {
+    const t1 = helper.makeItBlock('test-1', [1, 0, 5, 0]);
+    const t2 = helper.makeItBlock('test-2-${num}', [6, 0, 7, 0]);
+    const t3 = helper.makeItBlock('test-3-no-assertion', [8, 0, 10, 0]);
+    const d1 = helper.makeDescribeBlock('d-1', [t1, t2]);
+    const sourceRoot = helper.makeRoot([d1, t3]);
+
+    const a1 = helper.makeAssertion('test-1-a', 'KnownFail', ['d-1'], [0, 0]);
+    a1.fullName = undefined;
+    const a2 = helper.makeAssertion('test-2-100', 'KnownSuccess', ['d-1'], [7, 0]);
+    const matched = match.matchTestAssertions('a file', sourceRoot, [a1, a2]);
+
+    expect(matched).toHaveLength(3);
+    expect(matched.map((m) => m.name)).toEqual([
+      'test-3-no-assertion',
+      'test-1-a',
+      'd-1 test-2-100',
+    ]);
+    expect(matched.map((m) => m.status)).toEqual(['Unknown', 'KnownFail', 'KnownSuccess']);
+  });
+  it('duplicate name in the same block should generate warning', () => {
+    const t1 = helper.makeItBlock('test-1', [1, 0, 5, 0]);
+    const t2 = helper.makeItBlock('test-1', [6, 0, 7, 0]);
+    const sourceRoot = helper.makeRoot([t1, t2]);
+
+    const matched = match.matchTestAssertions('a file', sourceRoot, []);
+
+    expect(matched).toHaveLength(2);
+    expect(matched.map((m) => m.name)).toEqual(['test-1', 'test-1']);
+    expect(matched.map((m) => m.status)).toEqual(['Unknown', 'Unknown']);
+    expect(
+      mockWarn.mock.calls.find((call) => call[0].includes('duplicate names'))
+    ).not.toBeUndefined();
   });
 });

tests/diagnostics.test.ts

@@ -272,6 +272,9 @@ describe('test diagnostics', () => {
       const msg = 'a short error message';
       const testBlock: TestResult = {
         name: 'a',
+        names: {
+          src: 'a',
+        },
         start: { line: 2, column: 3 },
         end: { line: 4, column: 5 },
         lineNumberOfError: 3,

tests/helpers.test.ts

@@ -78,9 +78,9 @@ describe('ModuleHelpers', () => {
     };
 
     beforeEach(() => {
-      mockJoin.mockImplementation(require.requireActual('path').join);
-      mockNormalize.mockImplementation(require.requireActual('path').normalize);
-      mockExistsSync.mockImplementation(require.requireActual('path').existsSync);
+      mockJoin.mockImplementation(jest.requireActual('path').join);
+      mockNormalize.mockImplementation(jest.requireActual('path').normalize);
+      mockExistsSync.mockImplementation(jest.requireActual('path').existsSync);
     });
 
     it('returns "npm test --" when bootstrapped with create-react-app', () => {
@@ -110,14 +110,14 @@ describe('ModuleHelpers', () => {
     it('defaults to "node_modules/.bin/jest" when Jest is locally installed', () => {
       const expected = 'node_modules/.bin/jest.TEST';
 
-      mockJoin.mockImplementation(require.requireActual('path').posix.join);
+      mockJoin.mockImplementation(jest.requireActual('path').posix.join);
       mockNormalize.mockImplementation((arg) => arg);
       mockExistsSync.mockImplementation((path) => path === expected);
 
       expect(pathToJest(defaultSettings)).toBe(`"${expected}"`);
     });
     it('default jestToPath path can preserve special characters', () => {
-      mockJoin.mockImplementation(require.requireActual('path').posix.join);
+      mockJoin.mockImplementation(jest.requireActual('path').posix.join);
       mockNormalize.mockImplementation((arg) => arg);
 
       const testPaths = [
@@ -137,7 +137,7 @@ describe('ModuleHelpers', () => {
     it('defaults to "jest" when Jest is not locally installed', () => {
       const expected = '"jest.TEST"';
 
-      mockJoin.mockImplementation(require.requireActual('path').posix.join);
+      mockJoin.mockImplementation(jest.requireActual('path').posix.join);
       mockNormalize.mockImplementation((arg) => arg);
       mockExistsSync.mockImplementation(() => false);