Skip to content

Commit

Permalink
refactor: update errors (#2816)
Browse files Browse the repository at this point in the history
* perf: bypass source map handling, when using "resolves"/"rejects"

* chore: revert snapshot improvements

* test: fix test

* chore: fix initialization

* chore: fix snapshot
  • Loading branch information
sheremet-va authored Feb 11, 2023
1 parent ba821f9 commit fcd2df2
Show file tree
Hide file tree
Showing 14 changed files with 40 additions and 38 deletions.
2 changes: 1 addition & 1 deletion examples/mocks/test/error-mock.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ vi.mock('../src/default', () => {

test('when using top level variable, gives helpful message', async () => {
await expect(() => import('../src/default').then(m => m.default)).rejects
.toThrowErrorMatchingInlineSnapshot('"[vitest] There was an error, when mocking a module. If you are using \\"vi.mock\\" factory, make sure there are no top level variables inside, since this call is hoisted to top of the file. Read more: https://vitest.dev/api/#vi-mock"')
.toThrowErrorMatchingInlineSnapshot('"[vitest] There was an error when mocking a module. If you are using \\"vi.mock\\" factory, make sure there are no top level variables inside, since this call is hoisted to top of the file. Read more: https://vitest.dev/api/#vi-mock"')
})
File renamed without changes.
1 change: 1 addition & 0 deletions packages/utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ export * from './random'
export * from './display'
export * from './constants'
export * from './colors'
export * from './error'
6 changes: 3 additions & 3 deletions packages/vitest/src/api/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { API_PATH } from '../constants'
import type { Vitest } from '../node'
import type { File, ModuleGraphData, Reporter, TaskResultPack, UserConsoleLog } from '../types'
import { getModuleGraph } from '../utils'
import { parseStacktrace } from '../utils/source-map'
import { parseErrorStacktrace } from '../utils/source-map'
import type { TransformResultWithSource, WebSocketEvents, WebSocketHandlers } from './types'

export function setup(ctx: Vitest) {
Expand Down Expand Up @@ -140,9 +140,9 @@ class WebSocketReporter implements Reporter {
packs.forEach(([, result]) => {
// TODO remove after "error" deprecation is removed
if (result?.error)
result.error.stacks = parseStacktrace(result.error)
result.error.stacks = parseErrorStacktrace(result.error)
result?.errors?.forEach((error) => {
error.stacks = parseStacktrace(error)
error.stacks = parseErrorStacktrace(error)
})
})

Expand Down
7 changes: 2 additions & 5 deletions packages/vitest/src/integrations/snapshot/port/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@

import type { OptionsReceived as PrettyFormatOptions } from 'pretty-format'
import type { ParsedStack, SnapshotData, SnapshotMatchOptions, SnapshotResult, SnapshotStateOptions, SnapshotUpdateState } from '../../../types'
import { slash } from '../../../utils'
import { parseStacktrace } from '../../../utils/source-map'
import { parseErrorStacktrace } from '../../../utils/source-map'
import type { SnapshotEnvironment } from '../env'
import { getSnapshotEnvironment } from '../env'
import type { InlineSnapshot } from './inlineSnapshot'
Expand Down Expand Up @@ -122,9 +121,7 @@ export default class SnapshotState {
): void {
this._dirty = true
if (options.isInline) {
const error = options.error || new Error('Unknown error')
const stacks = parseStacktrace(error, true)
stacks.forEach(i => i.file = slash(i.file))
const stacks = parseErrorStacktrace(options.error || new Error('snapshot'), true)
const stack = this._inferInlineSnapshotStack(stacks)
if (!stack) {
throw new Error(
Expand Down
9 changes: 3 additions & 6 deletions packages/vitest/src/integrations/vi.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import type { FakeTimerInstallOpts } from '@sinonjs/fake-timers'
import { createSimpleStackTrace } from '@vitest/utils'
import { parseSingleStack } from '../utils/source-map'
import type { VitestMocker } from '../runtime/mocker'
import type { ResolvedConfig, RuntimeConfig } from '../types'
import { getWorkerState, resetModules, waitForImportsToResolve } from '../utils'
import type { MockFactoryWithHelper } from '../types/mocker'
import { createSimpleStackTrace } from '../utils/error'
import { FakeTimers } from './mock/timers'
import type { EnhancedSpy, MaybeMocked, MaybeMockedDeep, MaybePartiallyMocked, MaybePartiallyMockedDeep } from './spy'
import { fn, isMockFunction, spies, spyOn } from './spy'
Expand All @@ -21,11 +21,8 @@ class VitestUtils {

if (!this._mocker) {
const errorMsg = 'Vitest was initialized with native Node instead of Vite Node.'
+ '\n\nOne of the following is possible:'
+ '\n- "vitest" is imported outside of your tests (in that case, use "vitest/node" or import.meta.vitest)'
+ '\n- "vitest" is imported inside "globalSetup" (use "setupFiles", because "globalSetup" runs in a different context)'
+ '\n- Your dependency inside "node_modules" imports "vitest" directly (in that case, inline that dependency, using "deps.inline" config)'
+ '\n- Otherwise, it might be a Vitest bug. Please report it to https://github.com/vitest-dev/vitest/issues\n'
+ '\n\nIt\'s possible that you are importing "vitest" directly inside "globalSetup". In that case, use "setupFiles" because "globalSetup" runs in a different context.'
+ '\nOtherwise, it might be a Vitest bug. Please report it to https://github.com/vitest-dev/vitest/issues\n'
throw new Error(errorMsg)
}

Expand Down
5 changes: 2 additions & 3 deletions packages/vitest/src/node/core.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { existsSync, promises as fs } from 'node:fs'
import type { ViteDevServer } from 'vite'
import { normalizePath } from 'vite'
import { relative, toNamespacedPath } from 'pathe'
import { normalize, relative, toNamespacedPath } from 'pathe'
import fg from 'fast-glob'
import mm from 'micromatch'
import c from 'picocolors'
Expand Down Expand Up @@ -98,7 +97,7 @@ export class Vitest {

// since we set `server.hmr: false`, Vite does not auto restart itself
server.watcher.on('change', async (file) => {
file = normalizePath(file)
file = normalize(file)
const isConfig = file === server.config.configFile
if (isConfig) {
await Promise.all(this._onRestartListeners.map(fn => fn('config')))
Expand Down
4 changes: 2 additions & 2 deletions packages/vitest/src/node/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import cliTruncate from 'cli-truncate'
import { type DiffOptions, unifiedDiff } from '@vitest/utils/diff'
import { stringify } from '@vitest/utils'
import type { ErrorWithDiff, ParsedStack } from '../types'
import { lineSplitRE, parseStacktrace, positionToOffset } from '../utils/source-map'
import { lineSplitRE, parseErrorStacktrace, positionToOffset } from '../utils/source-map'
import { F_POINTER } from '../utils/figures'
import { TypeCheckError } from '../typecheck/typechecker'
import type { Vitest } from './core'
Expand Down Expand Up @@ -38,7 +38,7 @@ export async function printError(error: unknown, ctx: Vitest, options: PrintErro
} as any
}

const stacks = parseStacktrace(e, fullStack)
const stacks = parseErrorStacktrace(e, fullStack)

const nearest = error instanceof TypeCheckError
? error.stacks[0]
Expand Down
4 changes: 2 additions & 2 deletions packages/vitest/src/node/plugins/mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ export const MocksPlugin = (): Plugin => {

// if no vitest import found, check if the mock API is reachable after the hoisting
if (!found) {
m.prepend('try { vi } catch (_) { try { vitest } catch (__)'
+ `{ throw new Error(${JSON.stringify(API_NOT_FOUND_ERROR)}) } }\n`)
m.prepend('if (typeof globalThis.vi === "undefined" && typeof globalThis.vitest === "undefined") '
+ `{ throw new Error(${JSON.stringify(API_NOT_FOUND_ERROR)}) }\n`)
}

return {
Expand Down
4 changes: 2 additions & 2 deletions packages/vitest/src/node/reporters/json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { Vitest } from '../../node'
import type { File, Reporter, Suite, Task, TaskState } from '../../types'
import { getSuites, getTests } from '../../utils'
import { getOutputFile } from '../../utils/config-helpers'
import { parseStacktrace } from '../../utils/source-map'
import { parseErrorStacktrace } from '../../utils/source-map'

// for compatibility reasons, the reporter produces a JSON similar to the one produced by the Jest JSON reporter
// the following types are extracted from the Jest repository (and simplified)
Expand Down Expand Up @@ -183,7 +183,7 @@ export class JsonReporter implements Reporter {
if (!error)
return

const stack = parseStacktrace(error)
const stack = parseErrorStacktrace(error)
const frame = stack[stack.length - 1]
if (!frame)
return
Expand Down
4 changes: 2 additions & 2 deletions packages/vitest/src/node/reporters/junit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type { Task } from '@vitest/runner'
import type { ErrorWithDiff } from '@vitest/runner/utils'
import type { Vitest } from '../../node'
import type { Reporter } from '../../types/reporter'
import { parseStacktrace } from '../../utils/source-map'
import { parseErrorStacktrace } from '../../utils/source-map'
import { F_POINTER } from '../../utils/figures'
import { getOutputFile } from '../../utils/config-helpers'
import { IndentedLogger } from './renderers/indented-logger'
Expand Down Expand Up @@ -125,7 +125,7 @@ export class JUnitReporter implements Reporter {
// Be sure to escape any XML in the error Details
await this.baseLog(escapeXML(errorDetails))

const stack = parseStacktrace(error)
const stack = parseErrorStacktrace(error)

// TODO: This is same as printStack but without colors. Find a way to reuse code.
for (const frame of stack) {
Expand Down
4 changes: 2 additions & 2 deletions packages/vitest/src/node/reporters/tap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { Task } from '@vitest/runner'
import type { ParsedStack } from '@vitest/runner/utils'
import type { Vitest } from '../../node'
import type { Reporter } from '../../types/reporter'
import { parseStacktrace } from '../../utils/source-map'
import { parseErrorStacktrace } from '../../utils/source-map'
import { IndentedLogger } from './renderers/indented-logger'

function yamlString(str: string): string {
Expand Down Expand Up @@ -72,7 +72,7 @@ export class TapReporter implements Reporter {
this.logger.indent()

task.result.errors.forEach((error) => {
const stacks = parseStacktrace(error)
const stacks = parseErrorStacktrace(error)
const stack = stacks[0]

this.logger.log('---')
Expand Down
4 changes: 3 additions & 1 deletion packages/vitest/src/runtime/mocker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ export class VitestMocker {
}
catch (err) {
const vitestError = new Error(
'[vitest] There was an error, when mocking a module. '
'[vitest] There was an error when mocking a module. '
+ 'If you are using "vi.mock" factory, make sure there are no top level variables inside, since this call is hoisted to top of the file. '
+ 'Read more: https://vitest.dev/api/#vi-mock')
vitestError.cause = err
Expand All @@ -133,6 +133,8 @@ export class VitestMocker {
return target.then.bind(target)
}
else if (!(prop in target)) {
if (prop === '__esModule')
return undefined
const c = getColors()
throw new Error(
`[vitest] No "${String(prop)}" export is defined on the "${mockpath}" mock. `
Expand Down
24 changes: 15 additions & 9 deletions packages/vitest/src/utils/source-map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,8 @@ export function parseSingleStack(raw: string): ParsedStack | null {
}
}

export function parseStacktrace(e: ErrorWithDiff, full = false): ParsedStack[] {
if (!e)
return []

if (e.stacks)
return e.stacks

const stackStr = e.stack || e.stackStr || ''
const stackFrames = stackStr
export function parseStacktrace(stack: string, full = false): ParsedStack[] {
const stackFrames = stack
.split('\n')
.map((raw): ParsedStack | null => {
const stack = parseSingleStack(raw)
Expand All @@ -95,6 +88,19 @@ export function parseStacktrace(e: ErrorWithDiff, full = false): ParsedStack[] {
})
.filter(notNullish)

return stackFrames
}

export function parseErrorStacktrace(e: ErrorWithDiff, full = false): ParsedStack[] {
if (!e)
return []

if (e.stacks)
return e.stacks

const stackStr = e.stack || e.stackStr || ''
const stackFrames = parseStacktrace(stackStr, full)

e.stacks = stackFrames
return stackFrames
}
Expand Down

0 comments on commit fcd2df2

Please sign in to comment.