Fix some potentially misleading tests that were passing

Author: leeyi45

src/modules/preprocessor/__tests__/linker.ts

@@ -5,9 +5,10 @@ import { CircularImportError, ModuleNotFoundError } from '../../errors'
 import type { SourceFiles } from '../../moduleTypes'
 import parseProgramsAndConstructImportGraph from '../linker'
 
-import * as resolver from '../resolver'
-import { assertNodeType, assertTrue } from '../../../utils/testing/misc'
-jest.spyOn(resolver, 'default')
+import * as parser from '../../../parser/parser'
+import { asMockedFunc, assertNodeType, assertTrue } from '../../../utils/testing/misc'
+
+jest.spyOn(parser, 'parse')
 
 beforeEach(() => {
   jest.clearAllMocks()
@@ -35,6 +36,22 @@ async function expectError<T extends SourceFiles>(files: T, entrypointFilePath:
   return context.errors
 }
 
+function checkCallsToParser(yesCalls: string[], noCalls: string[]) {
+  const { calls } = asMockedFunc(parser.parse).mock
+  const toPaths = calls.map(([, , options]) => {
+    const path = options?.sourceFile
+    return path
+  })
+
+  for (const each of yesCalls) {
+    expect(toPaths).toContainEqual(each)
+  }
+
+  for (const each of noCalls) {
+    expect(toPaths).not.toContainEqual(each)
+  }
+}
+
 test('Adds CircularImportError and returns undefined when imports are circular', async () => {
   const [error] = await expectError(
     {
@@ -63,17 +80,16 @@ test.skip('Longer cycle causes also causes CircularImportError', async () => {
   )
 
   expect(error).toBeInstanceOf(CircularImportError)
-  expect(resolver.default).not.toHaveBeenCalledWith('./e.js')
 })
 
 test('Self Circular Imports cause a short circuiting of the linker', async () => {
   const [error] = await expectError(
     {
-      '/a.js': 'import { a } from "./a.js";',
-      '/c.js': `
+      '/a.js': `
+        import { a } from "./a.js";
         import { b } from "./b.js";
-        export const c = "c";
       `,
+      '/c.js': `export const c = "c";`,
       '/d.js': `
         import { a } from "./a.js";
         import { c } from "./c.js";
@@ -83,43 +99,9 @@ test('Self Circular Imports cause a short circuiting of the linker', async () =>
   )
 
   expect(error).toBeInstanceOf(CircularImportError)
-  expect(resolver.default).not.toHaveBeenCalledWith('./c.js')
-  expect(resolver.default).not.toHaveBeenCalledWith('./b.js')
-})
-
-test('Parse errors cause a short circuiting of the linker', async () => {
-  const [error] = await expectError(
-    {
-      '/a.js': 'export const a = "a";',
-      '/b.js': `
-      import { a } from "./a.js";
-      export function b() {
-        return a
-      }
-    `,
-      '/c.js': 'import { b } from "./b.js";'
-    },
-    '/b.js'
-  )
-  expect(error).toBeInstanceOf(MissingSemicolonError)
-  expect(resolver.default).not.toHaveBeenCalledWith('./a.js')
-})
-
-test('ModuleNotFoundErrors short circuit the linker', async () => {
-  const [error] = await expectError(
-    {
-      '/a.js': 'export const a = "a";',
-      '/b.js': `
-      import { c } from './c.js';
-      import { a } from './a.js';
-    `,
-      '/d.js': 'import { b } from "./b.js";'
-    },
-    '/d.js'
-  )
-
-  expect(error).toBeInstanceOf(ModuleNotFoundError)
-  expect(resolver.default).not.toHaveBeenCalledWith('./a.js')
+  // /b.js is only imported by the error throwing /a.js,
+  // so it should never be parsed
+  checkCallsToParser(['/a.js', '/c.js', '/d.js'], ['/b.js'])
 })
 
 test('Linker does tree-shaking', async () => {
@@ -133,7 +115,8 @@ test('Linker does tree-shaking', async () => {
 
   expect(errors.length).toEqual(0)
   assertTrue(result.ok)
-  expect(resolver.default).not.toHaveBeenCalledWith('./b.js')
+  // /a.js doesn't import /b.js, so it should not be parsed
+  checkCallsToParser(['/a.js'], ['/b.js'])
   expect(Object.keys(result.programs)).not.toContain('/b.js')
 })
 
@@ -144,7 +127,7 @@ test('Linker updates the source paths of Import Declaration nodes', async () =>
       '/b.js': `import { x } from "./dir0/a.js";
         export { x };
       `,
-      '/dir1/c.js': 'import { x } from "../b.js";',
+      '/dir1/c.js': 'import { x } from "../b.js";'
     },
     '/dir1/c.js'
   )
@@ -160,105 +143,129 @@ test('Linker updates the source paths of Import Declaration nodes', async () =>
 })
 
 describe('Test determining verbose errors', () => {
-   test('Verbose errors is normally false', async () => {
-    const [, result] = await testCode({
-      '/a.js': "const a = 0;"
-    }, '/a.js')
-    
+  test('Verbose errors is normally false', async () => {
+    const [, result] = await testCode(
+      {
+        '/a.js': 'const a = 0;'
+      },
+      '/a.js'
+    )
+
     assertTrue(result.ok)
     assertTrue(!result.verboseErrors)
   })
 
   test('Verbose errors enables normally', async () => {
-    const [, result] = await testCode({
-      '/a.js': "'enable verbose';"
-    }, '/a.js')
-    
+    const [, result] = await testCode(
+      {
+        '/a.js': "'enable verbose';"
+      },
+      '/a.js'
+    )
+
     assertTrue(result.ok)
     assertTrue(result.verboseErrors)
   })
 
   test('Verbose errors does not enable when it is not the entrypoint', async () => {
-    const [, result] = await testCode({
-      '/a.js': `
+    const [, result] = await testCode(
+      {
+        '/a.js': `
         'enable verbose';
         export const a = "a";
       `,
-      '/b.js': "import { a } from './a.js';"
-    }, '/b.js')
-    
+        '/b.js': "import { a } from './a.js';"
+      },
+      '/b.js'
+    )
+
     assertTrue(result.ok)
     assertTrue(!result.verboseErrors)
   })
 
   test('Verbose errors does not enable when it is not the first statement', async () => {
-    const [, result] = await testCode({
-      '/a.js': `
+    const [, result] = await testCode(
+      {
+        '/a.js': `
       export const a = "a";
       'enable verbose';
-      `,
-    }, '/a.js')
-    
+      `
+      },
+      '/a.js'
+    )
+
     assertTrue(result.ok)
     assertTrue(!result.verboseErrors)
   })
 
   test('Verbose errors does not enable when it is an unknown entrypoint', async () => {
-    const [, result] = await testCode({
-      '/a.js': `
+    const [, result] = await testCode(
+      {
+        '/a.js': `
       export const a = "a";
       'enable verbose';
-      `,
-    }, '/d.js' as any)
-    
+      `
+      },
+      '/d.js' as any
+    )
+
     assertTrue(!result.ok)
     assertTrue(!result.verboseErrors)
   })
 
   test('Verbose errors enables even if other files have parse errors', async () => {
-    const [{ errors }, result] = await testCode({
-      '/a.js': `
+    const [{ errors }, result] = await testCode(
+      {
+        '/a.js': `
       'enable verbose';
       import { b } from "./b.js";
       `,
-      '/b.js': `export const b = "b"`
-    }, '/a.js')
-    
+        '/b.js': `export const b = "b"`
+      },
+      '/a.js'
+    )
+
     assertTrue(!result.ok)
     assertTrue(result.verboseErrors)
     expect(errors.length).toEqual(1)
     expect(errors[0]).toBeInstanceOf(MissingSemicolonError)
   })
 
   test('Verbose errors enables even if the entrypoint has parse errors', async () => {
-    const [{ errors }, result] = await testCode({
-      '/a.js': `
+    const [{ errors }, result] = await testCode(
+      {
+        '/a.js': `
       'enable verbose';
       const x = 0
-      `,
-    }, '/a.js')
-    
+      `
+      },
+      '/a.js'
+    )
+
     assertTrue(!result.ok)
     assertTrue(result.verboseErrors)
     expect(errors.length).toEqual(1)
     expect(errors[0]).toBeInstanceOf(MissingSemicolonError)
   })
 
   test('Verbose errors enables even if there is a module resolution problem', async () => {
-    const [{ errors }, result] = await testCode({
-      '/a.js': `
+    const [{ errors }, result] = await testCode(
+      {
+        '/a.js': `
       'enable verbose';
       import { b } from "./b.js";
       `,
-     '/b.js': `
+        '/b.js': `
         import { c } from "./c.js";
         export { c as b };
       `
-    }, '/a.js')
-    
+      },
+      '/a.js'
+    )
+
     assertTrue(!result.ok)
     assertTrue(result.verboseErrors)
     expect(errors.length).toEqual(1)
     expect(errors[0]).toBeInstanceOf(ModuleNotFoundError)
   })
-})
+})

src/modules/preprocessor/__tests__/preprocessor.ts

@@ -424,9 +424,17 @@ describe('preprocessFileImports', () => {
   it('catches errors thrown by the bundler', async () => {
     const context = mockContext(Chapter.SOURCE_4)
     const errorToThrow = new Error()
-    const promise = preprocessFileImports(wrapFiles({ 
-      '/a.js': 'const a = 0;',
-    }), '/a.js', context, {}, () => { throw errorToThrow })
+    const promise = preprocessFileImports(
+      wrapFiles({
+        '/a.js': 'const a = 0;'
+      }),
+      '/a.js',
+      context,
+      {},
+      () => {
+        throw errorToThrow
+      }
+    )
 
     await expect(promise).resolves.not.toThrow()
     expect(context.errors[0]).toBe(errorToThrow)
@@ -435,10 +443,18 @@ describe('preprocessFileImports', () => {
   it('catches import analysis errors', async () => {
     const context = mockContext(Chapter.SOURCE_4)
     const errorToThrow = new Error()
-    const promise = preprocessFileImports(wrapFiles({ 
-      '/a.js': `import { b } from "./b.js";`,
-      '/b.js': 'export const a = "b";'
-    }), '/a.js', context, {}, () => { throw errorToThrow })
+    const promise = preprocessFileImports(
+      wrapFiles({
+        '/a.js': `import { b } from "./b.js";`,
+        '/b.js': 'export const a = "b";'
+      }),
+      '/a.js',
+      context,
+      {},
+      () => {
+        throw errorToThrow
+      }
+    )
 
     await expect(promise).resolves.not.toThrow()
     expect(context.errors[0]).toBeInstanceOf(UndefinedImportError)