From de59e3f1f26ff37869d33ec74f104b5ea31a5c1b Mon Sep 17 00:00:00 2001 From: Ahn Date: Thu, 30 Apr 2020 08:14:19 +0200 Subject: [PATCH] test: correct unit tests and e2e tests for language service (#1579) --- .../__snapshots__/diagnostics.test.ts.snap | 156 +----------------- e2e/__tests__/diagnostics.test.ts | 51 +----- src/__helpers__/fakers.ts | 3 +- .../{ => changed-modules}/main.spec.ts | 0 src/__mocks__/{ => changed-modules}/main.ts | 0 src/__mocks__/unchanged-modules/main.spec.ts | 3 + src/__mocks__/unchanged-modules/main.ts | 3 + .../language-service.spec.ts.snap | 2 + src/compiler/compiler-utils.ts | 4 - src/compiler/language-service.spec.ts | 64 ++++++- src/compiler/language-service.ts | 5 +- 11 files changed, 81 insertions(+), 210 deletions(-) rename src/__mocks__/{ => changed-modules}/main.spec.ts (100%) rename src/__mocks__/{ => changed-modules}/main.ts (100%) create mode 100644 src/__mocks__/unchanged-modules/main.spec.ts create mode 100644 src/__mocks__/unchanged-modules/main.ts diff --git a/e2e/__tests__/__snapshots__/diagnostics.test.ts.snap b/e2e/__tests__/__snapshots__/diagnostics.test.ts.snap index 312773b49e..cb930f3571 100644 --- a/e2e/__tests__/__snapshots__/diagnostics.test.ts.snap +++ b/e2e/__tests__/__snapshots__/diagnostics.test.ts.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`With diagnostics throw first throw should fail using template "default" 1`] = ` +exports[`With diagnostics throw should fail using template "default" 1`] = ` × jest ↳ exit code: 1 ===[ STDOUT ]=================================================================== @@ -27,7 +27,7 @@ exports[`With diagnostics throw first throw should fail using template "default" ================================================================================ `; -exports[`With diagnostics throw first throw should fail using template "with-babel-7" 1`] = ` +exports[`With diagnostics throw should fail using template "with-babel-7" 1`] = ` × jest ↳ exit code: 1 ===[ STDOUT ]=================================================================== @@ -54,7 +54,7 @@ exports[`With diagnostics throw first throw should fail using template "with-bab ================================================================================ `; -exports[`With diagnostics throw first throw should fail using template "with-babel-7-string-config" 1`] = ` +exports[`With diagnostics throw should fail using template "with-babel-7-string-config" 1`] = ` × jest ↳ exit code: 1 ===[ STDOUT ]=================================================================== @@ -81,156 +81,6 @@ exports[`With diagnostics throw first throw should fail using template "with-bab ================================================================================ `; -exports[`With diagnostics throw then fail when code has changed to invalid base on cache of the previous run should fail using template "default" 1`] = ` - × jest - ↳ exit code: 1 - ===[ STDOUT ]=================================================================== - - ===[ STDERR ]=================================================================== - FAIL ./main.spec.ts - × foo is 42 - - ● foo is 42 - - expect(received).toBe(expected) // Object.is equality - - Expected: 42 - Received: 43 - - 4 | - 5 | test('foo is 42', () => { - > 6 | expect(foo).toBe(42); - | ^ - 7 | }); - 8 | - - at Object. (main.spec.ts:6:15) - - Test Suites: 1 failed, 1 total - Tests: 1 failed, 1 total - Snapshots: 0 total - Time: XXs - Ran all test suites. - ================================================================================ -`; - -exports[`With diagnostics throw then fail when code has changed to invalid base on cache of the previous run should fail using template "with-babel-7" 1`] = ` - × jest - ↳ exit code: 1 - ===[ STDOUT ]=================================================================== - - ===[ STDERR ]=================================================================== - FAIL ./main.spec.ts - × foo is 42 - - ● foo is 42 - - expect(received).toBe(expected) // Object.is equality - - Expected: 42 - Received: 43 - - 4 | - 5 | test('foo is 42', () => { - > 6 | expect(foo).toBe(42); - | ^ - 7 | }); - 8 | - - at Object. (main.spec.ts:6:15) - - Test Suites: 1 failed, 1 total - Tests: 1 failed, 1 total - Snapshots: 0 total - Time: XXs - Ran all test suites. - ================================================================================ -`; - -exports[`With diagnostics throw then fail when code has changed to invalid base on cache of the previous run should fail using template "with-babel-7-string-config" 1`] = ` - × jest - ↳ exit code: 1 - ===[ STDOUT ]=================================================================== - - ===[ STDERR ]=================================================================== - FAIL ./main.spec.ts - × foo is 42 - - ● foo is 42 - - expect(received).toBe(expected) // Object.is equality - - Expected: 42 - Received: 43 - - 4 | - 5 | test('foo is 42', () => { - > 6 | expect(foo).toBe(42); - | ^ - 7 | }); - 8 | - - at Object. (main.spec.ts:6:15) - - Test Suites: 1 failed, 1 total - Tests: 1 failed, 1 total - Snapshots: 0 total - Time: XXs - Ran all test suites. - ================================================================================ -`; - -exports[`With diagnostics throw then pass when type has changed to valid base on cache of the previous run should pass using template "default" 1`] = ` - √ jest - ↳ exit code: 0 - ===[ STDOUT ]=================================================================== - - ===[ STDERR ]=================================================================== - PASS ./main.spec.ts - √ foo is 42 - - Test Suites: 1 passed, 1 total - Tests: 1 passed, 1 total - Snapshots: 0 total - Time: XXs - Ran all test suites. - ================================================================================ -`; - -exports[`With diagnostics throw then pass when type has changed to valid base on cache of the previous run should pass using template "with-babel-7" 1`] = ` - √ jest - ↳ exit code: 0 - ===[ STDOUT ]=================================================================== - - ===[ STDERR ]=================================================================== - PASS ./main.spec.ts - √ foo is 42 - - Test Suites: 1 passed, 1 total - Tests: 1 passed, 1 total - Snapshots: 0 total - Time: XXs - Ran all test suites. - ================================================================================ -`; - -exports[`With diagnostics throw then pass when type has changed to valid base on cache of the previous run should pass using template "with-babel-7-string-config" 1`] = ` - √ jest - ↳ exit code: 0 - ===[ STDOUT ]=================================================================== - - ===[ STDERR ]=================================================================== - PASS ./main.spec.ts - √ foo is 42 - - Test Suites: 1 passed, 1 total - Tests: 1 passed, 1 total - Snapshots: 0 total - Time: XXs - Ran all test suites. - ================================================================================ -`; - exports[`With diagnostics warn only should pass using template "default" 1`] = ` √ jest --no-cache ↳ exit code: 0 diff --git a/e2e/__tests__/diagnostics.test.ts b/e2e/__tests__/diagnostics.test.ts index 25525ef7d2..451d472313 100644 --- a/e2e/__tests__/diagnostics.test.ts +++ b/e2e/__tests__/diagnostics.test.ts @@ -1,55 +1,14 @@ -import { writeFileSync } from 'fs' -import { join } from 'path' - import { allValidPackageSets } from '../__helpers__/templates' import { configureTestCase } from '../__helpers__/test-case' describe('With diagnostics throw', () => { const testCase = configureTestCase('diagnostics/throw') - describe('first throw', () => { - testCase.runWithTemplates(allValidPackageSets, 1, (runTest, { testLabel }) => { - it(testLabel, () => { - const result = runTest() - expect(result.status).toBe(1) - expect(result).toMatchSnapshot() - }) - }) - }) - - describe('then pass when type has changed to valid base on cache of the previous run', () => { - beforeAll(() => { - writeFileSync(join(__dirname, '../__cases__/diagnostics/throw/main.ts'), `export const foo = 42\nexport type Thing = { a: number }`) - }) - - afterAll(() => { - writeFileSync(join(__dirname, '../__cases__/diagnostics/throw/main.ts'), `export const foo = 42\nexport type Thing = { a: number, b: number }\n`) - }) - - testCase.runWithTemplates(allValidPackageSets, 0, (runTest, { testLabel }) => { - it(testLabel, () => { - const result = runTest() - expect(result.status).toBe(0) - expect(result).toMatchSnapshot() - }) - }) - }) - - describe('then fail when code has changed to invalid base on cache of the previous run', () => { - beforeAll(() => { - writeFileSync(join(__dirname, '../__cases__/diagnostics/throw/main.ts'), `export const foo = 43\nexport type Thing = { a: number }`) - }) - - afterAll(() => { - writeFileSync(join(__dirname, '../__cases__/diagnostics/throw/main.ts'), `export const foo = 42\nexport type Thing = { a: number, b: number }\n`) - }) - - testCase.runWithTemplates(allValidPackageSets, 1, (runTest, { testLabel }) => { - it(testLabel, () => { - const result = runTest() - expect(result.status).toBe(1) - expect(result).toMatchSnapshot() - }) + testCase.runWithTemplates(allValidPackageSets, 1, (runTest, { testLabel }) => { + it(testLabel, () => { + const result = runTest() + expect(result.status).toBe(1) + expect(result).toMatchSnapshot() }) }) }) diff --git a/src/__helpers__/fakers.ts b/src/__helpers__/fakers.ts index 210e129c24..022af486da 100644 --- a/src/__helpers__/fakers.ts +++ b/src/__helpers__/fakers.ts @@ -68,9 +68,10 @@ export function makeCompiler({ pretty: false, } const testRegex = ['^.+\\.[tj]sx?$'] + const testMatch = ['^.+\\.tsx?$'] jestConfig = { ...jestConfig, - testMatch: ['^.+\\.tsx?$'], + testMatch: jestConfig?.testMatch ? [...jestConfig.testMatch, ...testMatch] : testMatch, testRegex: jestConfig?.testRegex ? [...testRegex, ...jestConfig.testRegex] : testRegex, } const cs = new ConfigSet(getJestConfig(jestConfig, tsJestConfig), parentConfig) diff --git a/src/__mocks__/main.spec.ts b/src/__mocks__/changed-modules/main.spec.ts similarity index 100% rename from src/__mocks__/main.spec.ts rename to src/__mocks__/changed-modules/main.spec.ts diff --git a/src/__mocks__/main.ts b/src/__mocks__/changed-modules/main.ts similarity index 100% rename from src/__mocks__/main.ts rename to src/__mocks__/changed-modules/main.ts diff --git a/src/__mocks__/unchanged-modules/main.spec.ts b/src/__mocks__/unchanged-modules/main.spec.ts new file mode 100644 index 0000000000..40e5da46d6 --- /dev/null +++ b/src/__mocks__/unchanged-modules/main.spec.ts @@ -0,0 +1,3 @@ +import { Thing } from './main' + +export const thing: Thing = { a: 1 } diff --git a/src/__mocks__/unchanged-modules/main.ts b/src/__mocks__/unchanged-modules/main.ts new file mode 100644 index 0000000000..fea8e002b1 --- /dev/null +++ b/src/__mocks__/unchanged-modules/main.ts @@ -0,0 +1,3 @@ +export interface Thing { + a: number +} diff --git a/src/compiler/__snapshots__/language-service.spec.ts.snap b/src/compiler/__snapshots__/language-service.spec.ts.snap index 4c8fb3d4f2..7ba8371626 100644 --- a/src/compiler/__snapshots__/language-service.spec.ts.snap +++ b/src/compiler/__snapshots__/language-service.spec.ts.snap @@ -84,6 +84,8 @@ exports[`Language service should compile tsx file for other jsx options 1`] = ` ================================================================================ `; +exports[`Language service should do type check for the test file when imported module has changed 1`] = `"src/__mocks__/changed-modules/main.spec.ts(3,14): error TS2741: Property 'b' is missing in type '{ a: number; }' but required in type 'Thing'."`; + exports[`Language service should report diagnostics related to typings with pathRegex config matches file name 1`] = `"test-match-regex-diagnostics.ts(3,7): error TS2322: Type 'number' is not assignable to type 'string'."`; exports[`Language service should throw error when cannot compile 1`] = ` diff --git a/src/compiler/compiler-utils.ts b/src/compiler/compiler-utils.ts index 3666a714b2..154c6f911d 100644 --- a/src/compiler/compiler-utils.ts +++ b/src/compiler/compiler-utils.ts @@ -9,10 +9,6 @@ import { EXTENSION_REGEX, JSON_REGEX, TS_TSX_REGEX } from '../constants' import { MemoryCache, SourceOutput, TSFiles } from '../types' import { sha1 } from '../util/sha1' -/** - * @internal - */ -export const hasOwn = Object.prototype.hasOwnProperty /** * @internal */ diff --git a/src/compiler/language-service.spec.ts b/src/compiler/language-service.spec.ts index 5bd671d8d9..133439d377 100644 --- a/src/compiler/language-service.spec.ts +++ b/src/compiler/language-service.spec.ts @@ -1,3 +1,5 @@ +import { LogLevels } from 'bs-logger' +import { readFileSync } from 'fs' import { removeSync, writeFileSync } from 'fs-extra' import { makeCompiler } from '../__helpers__/fakers' @@ -74,7 +76,7 @@ describe('Language service', () => { jestConfig: { cache: true, cacheDirectory: tmp, testRegex: [/.*\.(spec|test)\.[jt]sx?$/] as any[] }, tsJestConfig: { tsConfig: false }, }) - const fileName = 'src/__mocks__/main.spec.ts' + const fileName = 'src/__mocks__/unchanged-modules/main.spec.ts' const source = `import { Thing } from './main' export const thing: Thing = { a: 1 }` @@ -96,7 +98,7 @@ export const thing: Thing = { a: 1 }` jestConfig: { cache: true, cacheDirectory: tmp, testRegex: [/.*\.(foo|bar)\.[jt]sx?$/] as any[] }, tsJestConfig: { tsConfig: false }, }) - const fileName = 'src/__mocks__/main.spec.ts' + const fileName = 'src/__mocks__/unchanged-modules/main.spec.ts' const source = `import { Thing } from './main' export const thing: Thing = { a: 1 }` @@ -197,6 +199,64 @@ export const thing: Thing = { a: 1 }` removeSync(fileName) }) + it('should not do type check for the test file which is already finished type checking before', () => { + const tmp = tempDir('compiler') + const testFileName = 'src/__mocks__/unchanged-modules/main.spec.ts' + const testFileSrc = readFileSync(testFileName, 'utf-8') + const importedModuleSrc = readFileSync('src/__mocks__/unchanged-modules/main.ts', 'utf-8') + + const compiler = makeCompiler({ + jestConfig: { cache: true, cacheDirectory: tmp, testMatch: ['src/__mocks__/unchanged-modules/*.spec.ts'] }, + tsJestConfig: { tsConfig: false }, + }) + + compiler.compile(testFileSrc, testFileName) + logTarget.clear() + compiler.compile(importedModuleSrc, require.resolve('../__mocks__/unchanged-modules/main.ts')) + + expect(logTarget.filteredLines(LogLevels.debug, Infinity)).toMatchInlineSnapshot(` + Array [ + "[level:20] compileAndCacheResult(): get compile output + ", + "[level:20] compileFn(): compiling using language service + ", + "[level:20] updateMemoryCache(): update memory cache for language service + ", + "[level:20] visitSourceFileNode(): hoisting + ", + "[level:20] compileFn(): computing diagnostics using language service + ", + ] + `) + }) + + it('should do type check for the test file when imported module has changed', () => { + const tmp = tempDir('compiler') + const testFileName = 'src/__mocks__/changed-modules/main.spec.ts' + const testFileSrc = readFileSync(testFileName, 'utf-8') + const importedModulePath = 'src/__mocks__/changed-modules/main.ts' + const importedModuleSrc = readFileSync(importedModulePath, 'utf-8') + const newImportedModuleSrc = 'export interface Thing { a: number, b: number }' + + const compiler1 = makeCompiler({ + jestConfig: { cache: true, cacheDirectory: tmp, testMatch: ['src/__mocks__/changed-modules/*.spec.ts'] }, + tsJestConfig: { tsConfig: false }, + }) + compiler1.compile(testFileSrc, testFileName) + + writeFileSync(importedModulePath, 'export interface Thing { a: number, b: number }') + const compiler2 = makeCompiler({ + jestConfig: { cache: true, cacheDirectory: tmp, testMatch: ['src/__mocks__/changed-modules/*.spec.ts'] }, + tsJestConfig: { tsConfig: false }, + }) + + expect(() => + compiler2.compile(newImportedModuleSrc, require.resolve('../__mocks__/changed-modules/main.ts')), + ).toThrowErrorMatchingSnapshot() + + writeFileSync(importedModulePath, importedModuleSrc) + }) + it('should report diagnostics related to typings with pathRegex config matches file name', () => { const fileName = 'test-match-regex-diagnostics.ts' const source = ` diff --git a/src/compiler/language-service.ts b/src/compiler/language-service.ts index 9465d43447..e610d60a3e 100644 --- a/src/compiler/language-service.ts +++ b/src/compiler/language-service.ts @@ -12,7 +12,6 @@ import { cacheResolvedModules, getAndCacheProjectReference, getCompileResultFromReferencedProject, - hasOwn, isTestFile, } from './compiler-utils' @@ -163,7 +162,6 @@ export const initializeLanguageServiceInstance = ( if (isTestFile(configs.testMatchPatterns, fileName)) { cacheResolvedModules(fileName, code, memoryCache, service.getProgram()!, cacheDir, logger) } else { - /* istanbul ignore next (covered by e2e) */ Object.entries(memoryCache.resolvedModules) .filter(entry => { /** @@ -172,8 +170,7 @@ export const initializeLanguageServiceInstance = ( * test file for 1st time run after clearing cache because */ return ( - entry[1].modulePaths.find(modulePath => modulePath === fileName) && - !hasOwn.call(memoryCache.files, entry[0]) + entry[1].modulePaths.find(modulePath => modulePath === fileName) && !memoryCache.files.has(entry[0]) ) }) .forEach(entry => {