From 3573032875eac78eec55f37c7d42505bf2f0d4cb Mon Sep 17 00:00:00 2001 From: Vladimir Date: Fri, 17 Mar 2023 14:31:34 +0100 Subject: [PATCH] fix: show correct line numbers in stack trace when using vi.resetModules() (#3020) --- packages/vite-node/src/client.ts | 8 +++ packages/vitest/src/utils/index.ts | 4 +- test/core/test/utils.spec.ts | 63 +++++++++++-------- .../fixtures/reset-modules.test.ts | 18 ++++++ .../test/__snapshots__/runner.test.ts.snap | 13 +++- 5 files changed, 76 insertions(+), 30 deletions(-) create mode 100644 test/stacktraces/fixtures/reset-modules.test.ts diff --git a/packages/vite-node/src/client.ts b/packages/vite-node/src/client.ts index c85127937178..315e1c103553 100644 --- a/packages/vite-node/src/client.ts +++ b/packages/vite-node/src/client.ts @@ -92,6 +92,14 @@ export class ModuleCacheMap extends Map { return this.deleteByModuleId(this.normalizePath(fsPath)) } + invalidateModule(mod: ModuleCache) { + delete mod.evaluated + delete mod.resolving + delete mod.promise + delete mod.exports + return true + } + /** * Invalidate modules that dependent on the given modules, up to the main entry */ diff --git a/packages/vitest/src/utils/index.ts b/packages/vitest/src/utils/index.ts index 728d6fee66ea..66d5d86b28fb 100644 --- a/packages/vitest/src/utils/index.ts +++ b/packages/vitest/src/utils/index.ts @@ -31,10 +31,10 @@ export function resetModules(modules: ModuleCacheMap, resetMocks = false) { // don't clear mocks ...(!resetMocks ? [/^mock:/] : []), ] - modules.forEach((_, path) => { + modules.forEach((mod, path) => { if (skipPaths.some(re => re.test(path))) return - modules.delete(path) + modules.invalidateModule(mod) }) } diff --git a/test/core/test/utils.spec.ts b/test/core/test/utils.spec.ts index 3f9fa65eb49c..5e9fdc5a6251 100644 --- a/test/core/test/utils.spec.ts +++ b/test/core/test/utils.spec.ts @@ -1,8 +1,9 @@ -import { describe, expect, test } from 'vitest' +import { beforeAll, describe, expect, test } from 'vitest' import { assertTypes, deepClone, objectAttr, toArray } from '@vitest/utils' import { deepMerge, resetModules } from '../../../packages/vitest/src/utils' import { deepMergeSnapshot } from '../../../packages/vitest/src/integrations/snapshot/port/utils' -import type { ModuleCacheMap } from '../../../packages/vite-node/src/types' +import type { EncodedSourceMap } from '../../../packages/vite-node/src/types' +import { ModuleCacheMap } from '../../../packages/vite-node/dist/client' describe('assertTypes', () => { test('the type of value should be number', () => { @@ -151,36 +152,44 @@ describe('deepClone', () => { }) describe('resetModules doesn\'t resets only user modules', () => { - test('resets user modules', () => { - const moduleCache = new Map() as ModuleCacheMap - moduleCache.set('/some-module.ts', {}) - moduleCache.set('/@fs/some-path.ts', {}) - - resetModules(moduleCache) - - expect(moduleCache.size).toBe(0) - }) - - test('doesn\'t reset vitest modules', () => { - const moduleCache = new Map() as ModuleCacheMap - moduleCache.set('/node_modules/vitest/dist/index.js', {}) - moduleCache.set('/node_modules/vitest-virtual-da9876a/dist/index.js', {}) - moduleCache.set('/node_modules/some-module@vitest/dist/index.js', {}) - moduleCache.set('/packages/vitest/dist/index.js', {}) - + const mod = () => ({ evaluated: true, promise: Promise.resolve({}), resolving: false, exports: {}, map: {} as EncodedSourceMap }) + + const moduleCache = new ModuleCacheMap() + const modules = [ + ['/some-module.ts', true], + ['/@fs/some-path.ts', true], + ['/node_modules/vitest/dist/index.js', false], + ['/node_modules/vitest-virtual-da9876a/dist/index.js', false], + ['/node_modules/some-module@vitest/dist/index.js', false], + ['/packages/vitest/dist/index.js', false], + ['mock:/some-module.ts', false], + ['mock:/@fs/some-path.ts', false], + ] as const + + beforeAll(() => { + modules.forEach(([path]) => { + moduleCache.set(path, mod()) + }) resetModules(moduleCache) - - expect(moduleCache.size).toBe(4) }) - test('doesn\'t reset mocks', () => { - const moduleCache = new Map() as ModuleCacheMap - moduleCache.set('mock:/some-module.ts', {}) - moduleCache.set('mock:/@fs/some-path.ts', {}) + test.each(modules)('Cashe for %s is reseted (%s)', (path, reset) => { + const cached = moduleCache.get(path) - resetModules(moduleCache) + if (reset) { + expect(cached).not.toHaveProperty('evaluated') + expect(cached).not.toHaveProperty('resolving') + expect(cached).not.toHaveProperty('exports') + expect(cached).not.toHaveProperty('promise') + } + else { + expect(cached).toHaveProperty('evaluated') + expect(cached).toHaveProperty('resolving') + expect(cached).toHaveProperty('exports') + expect(cached).toHaveProperty('promise') + } - expect(moduleCache.size).toBe(2) + expect(cached).toHaveProperty('map') }) }) diff --git a/test/stacktraces/fixtures/reset-modules.test.ts b/test/stacktraces/fixtures/reset-modules.test.ts new file mode 100644 index 000000000000..884173d50e5a --- /dev/null +++ b/test/stacktraces/fixtures/reset-modules.test.ts @@ -0,0 +1,18 @@ +import { assert, describe, expect, it, vi } from 'vitest' + +describe('suite name', () => { + it('foo', () => { + vi.resetModules() + // a comment here + // another comment + // another comment + // another comment + // another comment + // another comment + // this will mess up the stacktrace lines + expect(1 + 1).eq(2) + expect(2 + 1).eq(3) + assert.equal(Math.sqrt(4), 2) + expect(Math.sqrt(4)).toBe(1) + }) +}) diff --git a/test/stacktraces/test/__snapshots__/runner.test.ts.snap b/test/stacktraces/test/__snapshots__/runner.test.ts.snap index 706d64998dc1..eb54c3593d60 100644 --- a/test/stacktraces/test/__snapshots__/runner.test.ts.snap +++ b/test/stacktraces/test/__snapshots__/runner.test.ts.snap @@ -1,4 +1,4 @@ -// Vitest Snapshot v1 +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html exports[`stacktraces should pick error frame if present > frame.spec.imba > frame.spec.imba 1`] = ` " FAIL frame.spec.imba [ frame.spec.imba ] @@ -43,3 +43,14 @@ exports[`stacktraces should respect sourcemaps > add-in-js.test.js > add-in-js.t 8| return expect(add(1, 2, 3)).toBe(6) " `; + +exports[`stacktraces should respect sourcemaps > reset-modules.test.ts > reset-modules.test.ts 1`] = ` +" ❯ reset-modules.test.ts:16:26 + 14| expect(2 + 1).eq(3) + 15| assert.equal(Math.sqrt(4), 2) + 16| expect(Math.sqrt(4)).toBe(1) + | ^ + 17| }) + 18| }) +" +`;