Skip to content

Commit

Permalink
fix: throw an error and a warning if .poll, .element, .rejects/…
Browse files Browse the repository at this point in the history
…`.resolves`, and `locator.*` weren't awaited (#6877)
  • Loading branch information
sheremet-va authored Nov 13, 2024
1 parent 9a0c93d commit 93b67c2
Show file tree
Hide file tree
Showing 24 changed files with 417 additions and 99 deletions.
6 changes: 5 additions & 1 deletion docs/api/expect.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ test('element exists', async () => {
```

::: warning
`expect.poll` makes every assertion asynchronous, so do not forget to await it otherwise you might get unhandled promise rejections.
`expect.poll` makes every assertion asynchronous, so you need to await it. Since Vitest 2.2, if you forget to await it, the test will fail with a warning to do so.

`expect.poll` doesn't work with several matchers:

Expand Down Expand Up @@ -1185,6 +1185,8 @@ test('buyApples returns new stock id', async () => {

:::warning
If the assertion is not awaited, then you will have a false-positive test that will pass every time. To make sure that assertions are actually called, you may use [`expect.assertions(number)`](#expect-assertions).

Since Vitest 2.2, if a method is not awaited, Vitest will show a warning at the end of the test. In Vitest 3, the test will be marked as "failed" if the assertion is not awaited.
:::

## rejects
Expand Down Expand Up @@ -1214,6 +1216,8 @@ test('buyApples throws an error when no id provided', async () => {

:::warning
If the assertion is not awaited, then you will have a false-positive test that will pass every time. To make sure that assertions were actually called, you can use [`expect.assertions(number)`](#expect-assertions).

Since Vitest 2.2, if a method is not awaited, Vitest will show a warning at the end of the test. In Vitest 3, the test will be marked as "failed" if the assertion is not awaited.
:::

## expect.assertions
Expand Down
2 changes: 2 additions & 0 deletions docs/guide/browser/locators.md
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,8 @@ It is recommended to use this only after the other locators don't work for your

## Methods

All methods are asynchronous and must be awaited. Since Vitest 2.2, tests will fail if a method is not awaited.

### click

```ts
Expand Down
80 changes: 44 additions & 36 deletions packages/browser/src/client/tester/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import type {
UserEventTabOptions,
UserEventTypeOptions,
} from '../../../context'
import { convertElementToCssSelector, getBrowserState, getWorkerState } from '../utils'
import { convertElementToCssSelector, ensureAwaited, getBrowserState, getWorkerState } from '../utils'

// this file should not import anything directly, only types and utils

Expand Down Expand Up @@ -40,12 +40,14 @@ export function createUserEvent(__tl_user_event_base__?: TestingLibraryUserEvent
return createUserEvent(__tl_user_event_base__, options)
},
async cleanup() {
if (typeof __tl_user_event_base__ !== 'undefined') {
__tl_user_event__ = __tl_user_event_base__?.setup(options ?? {})
return
}
await triggerCommand('__vitest_cleanup', keyboard)
keyboard.unreleased = []
return ensureAwaited(async () => {
if (typeof __tl_user_event_base__ !== 'undefined') {
__tl_user_event__ = __tl_user_event_base__?.setup(options ?? {})
return
}
await triggerCommand('__vitest_cleanup', keyboard)
keyboard.unreleased = []
})
},
click(element: Element | Locator, options: UserEventClickOptions = {}) {
return convertToLocator(element).click(processClickOptions(options))
Expand Down Expand Up @@ -84,39 +86,45 @@ export function createUserEvent(__tl_user_event_base__?: TestingLibraryUserEvent

// testing-library user-event
async type(element: Element | Locator, text: string, options: UserEventTypeOptions = {}) {
if (typeof __tl_user_event__ !== 'undefined') {
return __tl_user_event__.type(
element instanceof Element ? element : element.element(),
return ensureAwaited(async () => {
if (typeof __tl_user_event__ !== 'undefined') {
return __tl_user_event__.type(
element instanceof Element ? element : element.element(),
text,
options,
)
}

const selector = convertToSelector(element)
const { unreleased } = await triggerCommand<{ unreleased: string[] }>(
'__vitest_type',
selector,
text,
options,
{ ...options, unreleased: keyboard.unreleased },
)
}

const selector = convertToSelector(element)
const { unreleased } = await triggerCommand<{ unreleased: string[] }>(
'__vitest_type',
selector,
text,
{ ...options, unreleased: keyboard.unreleased },
)
keyboard.unreleased = unreleased
keyboard.unreleased = unreleased
})
},
tab(options: UserEventTabOptions = {}) {
if (typeof __tl_user_event__ !== 'undefined') {
return __tl_user_event__.tab(options)
}
return triggerCommand('__vitest_tab', options)
return ensureAwaited(() => {
if (typeof __tl_user_event__ !== 'undefined') {
return __tl_user_event__.tab(options)
}
return triggerCommand('__vitest_tab', options)
})
},
async keyboard(text: string) {
if (typeof __tl_user_event__ !== 'undefined') {
return __tl_user_event__.keyboard(text)
}
const { unreleased } = await triggerCommand<{ unreleased: string[] }>(
'__vitest_keyboard',
text,
keyboard,
)
keyboard.unreleased = unreleased
return ensureAwaited(async () => {
if (typeof __tl_user_event__ !== 'undefined') {
return __tl_user_event__.keyboard(text)
}
const { unreleased } = await triggerCommand<{ unreleased: string[] }>(
'__vitest_keyboard',
text,
keyboard,
)
keyboard.unreleased = unreleased
})
},
}
}
Expand Down Expand Up @@ -167,12 +175,12 @@ export const page: BrowserPage = {
const name
= options.path || `${taskName.replace(/[^a-z0-9]/gi, '-')}-${number}.png`

return triggerCommand('__vitest_screenshot', name, {
return ensureAwaited(() => triggerCommand('__vitest_screenshot', name, {
...options,
element: options.element
? convertToSelector(options.element)
: undefined,
})
}))
},
getByRole() {
throw new Error('Method "getByRole" is not implemented in the current provider.')
Expand Down
6 changes: 4 additions & 2 deletions packages/browser/src/client/tester/expect-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ export async function setupExpectDom() {
if (elementOrLocator instanceof Element || elementOrLocator == null) {
return elementOrLocator
}
const isNot = chai.util.flag(this, 'negate')
const name = chai.util.flag(this, '_name')
chai.util.flag(this, '_poll.element', true)

const isNot = chai.util.flag(this, 'negate') as boolean
const name = chai.util.flag(this, '_name') as string
// special case for `toBeInTheDocument` matcher
if (isNot && name === 'toBeInTheDocument') {
return elementOrLocator.query()
Expand Down
6 changes: 3 additions & 3 deletions packages/browser/src/client/tester/locators/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
Ivya,
type ParsedSelector,
} from 'ivya'
import { getBrowserState, getWorkerState } from '../../utils'
import { ensureAwaited, getBrowserState, getWorkerState } from '../../utils'
import { getElementError } from '../public-utils'

// we prefer using playwright locators because they are more powerful and support Shadow DOM
Expand Down Expand Up @@ -202,11 +202,11 @@ export abstract class Locator {
|| this.worker.current?.file?.filepath
|| undefined

return this.rpc.triggerCommand<T>(
return ensureAwaited(() => this.rpc.triggerCommand<T>(
this.state.contextId,
command,
filepath,
args,
)
))
}
}
20 changes: 10 additions & 10 deletions packages/browser/src/client/tester/locators/preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
getByTextSelector,
getByTitleSelector,
} from 'ivya'
import { convertElementToCssSelector } from '../../utils'
import { convertElementToCssSelector, ensureAwaited } from '../../utils'
import { getElementError } from '../public-utils'
import { Locator, selectorEngine } from './index'

Expand Down Expand Up @@ -58,28 +58,28 @@ class PreviewLocator extends Locator {
}

click(): Promise<void> {
return userEvent.click(this.element())
return ensureAwaited(() => userEvent.click(this.element()))
}

dblClick(): Promise<void> {
return userEvent.dblClick(this.element())
return ensureAwaited(() => userEvent.dblClick(this.element()))
}

tripleClick(): Promise<void> {
return userEvent.tripleClick(this.element())
return ensureAwaited(() => userEvent.tripleClick(this.element()))
}

hover(): Promise<void> {
return userEvent.hover(this.element())
return ensureAwaited(() => userEvent.hover(this.element()))
}

unhover(): Promise<void> {
return userEvent.unhover(this.element())
return ensureAwaited(() => userEvent.unhover(this.element()))
}

async fill(text: string): Promise<void> {
await this.clear()
return userEvent.type(this.element(), text)
return ensureAwaited(() => userEvent.type(this.element(), text))
}

async upload(file: string | string[] | File | File[]): Promise<void> {
Expand All @@ -100,7 +100,7 @@ class PreviewLocator extends Locator {
return fileInstance
})
const uploadFiles = await Promise.all(uploadPromise)
return userEvent.upload(this.element() as HTMLElement, uploadFiles)
return ensureAwaited(() => userEvent.upload(this.element() as HTMLElement, uploadFiles))
}

selectOptions(options_: string | string[] | HTMLElement | HTMLElement[] | Locator | Locator[]): Promise<void> {
Expand All @@ -110,15 +110,15 @@ class PreviewLocator extends Locator {
}
return option
})
return userEvent.selectOptions(this.element(), options as string[] | HTMLElement[])
return ensureAwaited(() => userEvent.selectOptions(this.element(), options as string[] | HTMLElement[]))
}

async dropTo(): Promise<void> {
throw new Error('The "preview" provider doesn\'t support `dropTo` method.')
}

clear(): Promise<void> {
return userEvent.clear(this.element())
return ensureAwaited(() => userEvent.clear(this.element()))
}

async screenshot(): Promise<never> {
Expand Down
6 changes: 2 additions & 4 deletions packages/browser/src/client/tester/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,8 @@ export function setupConsoleLogSpy() {
trace(...args)
const content = processLog(args)
const error = new Error('$$Trace')
const stack = (error.stack || '')
.split('\n')
.slice(error.stack?.includes('$$Trace') ? 2 : 1)
.join('\n')
const processor = (globalThis as any).__vitest_worker__?.onFilterStackTrace || ((s: string) => s || '')
const stack = processor(error.stack || '')
sendLog('stderr', `${content}\n${stack}`, true)
}

Expand Down
13 changes: 11 additions & 2 deletions packages/browser/src/client/tester/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ import { page, userEvent } from '@vitest/browser/context'
import { loadDiffConfig, loadSnapshotSerializers, takeCoverageInsideWorker } from 'vitest/browser'
import { NodeBenchmarkRunner, VitestTestRunner } from 'vitest/runners'
import { originalPositionFor, TraceMap } from 'vitest/utils'
import { executor } from '../utils'
import { createStackString, parseStacktrace } from '../../../../utils/src/source-map'
import { executor, getWorkerState } from '../utils'
import { rpc } from './rpc'
import { VitestBrowserSnapshotEnvironment } from './snapshot'

Expand All @@ -29,7 +30,7 @@ export function createBrowserRunner(
mocker: VitestBrowserClientMocker,
state: WorkerGlobalState,
coverageModule: CoverageHandler | null,
): { new (options: BrowserRunnerOptions): VitestRunner } {
): { new (options: BrowserRunnerOptions): VitestRunner & { sourceMapCache: Map<string, any> } } {
return class BrowserTestRunner extends runnerClass implements VitestRunner {
public config: SerializedConfig
hashMap = browserHashMap
Expand Down Expand Up @@ -171,6 +172,14 @@ export async function initiateRunner(
])
runner.config.diffOptions = diffOptions
cachedRunner = runner
getWorkerState().onFilterStackTrace = (stack: string) => {
const stacks = parseStacktrace(stack, {
getSourceMap(file) {
return runner.sourceMapCache.get(file)
},
})
return createStackString(stacks)
}
return runner
}

Expand Down
34 changes: 34 additions & 0 deletions packages/browser/src/client/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,40 @@ export function getConfig(): SerializedConfig {
return getBrowserState().config
}

export function ensureAwaited<T>(promise: () => Promise<T>): Promise<T> {
const test = getWorkerState().current
if (!test || test.type !== 'test') {
return promise()
}
let awaited = false
const sourceError = new Error('STACK_TRACE_ERROR')
test.onFinished ??= []
test.onFinished.push(() => {
if (!awaited) {
const error = new Error(
`The call was not awaited. This method is asynchronous and must be awaited; otherwise, the call will not start to avoid unhandled rejections.`,
)
error.stack = sourceError.stack?.replace(sourceError.message, error.message)
throw error
}
})
// don't even start the promise if it's not awaited to not cause any unhanded promise rejections
let promiseResult: Promise<T> | undefined
return {
then(onFulfilled, onRejected) {
awaited = true
return (promiseResult ||= promise()).then(onFulfilled, onRejected)
},
catch(onRejected) {
return (promiseResult ||= promise()).catch(onRejected)
},
finally(onFinally) {
return (promiseResult ||= promise()).finally(onFinally)
},
[Symbol.toStringTag]: 'Promise',
} satisfies Promise<T>
}

export interface BrowserRunnerState {
files: string[]
runningFiles: string[]
Expand Down
Loading

0 comments on commit 93b67c2

Please sign in to comment.