Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: throw an error and a warning if .poll, .element, .rejects/.resolves, and locator.* weren't awaited #6877

Merged
merged 16 commits into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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