Skip to content

Commit

Permalink
feat(waitFor): replace wait with waitFor
Browse files Browse the repository at this point in the history
Closes #376
Closes #416

BREAKING CHANGE: wait is now deprecated in favor of waitFor (which has basically the same API except it requires a callback and it also accepts optional mutation observer arguments).
BREAKING CHANGE: `waitForElement` is deprecated in favor of `find*` queries or `wait`.
BREAKING CHANGE: `waitForDomChange` is deprecated in favor of `wait`
BREAKING CHANGE: default timeout for async utilities is now 1000ms rather than 4500ms. This can be configured: https://testing-library.com/docs/dom-testing-library/api-configuration
  • Loading branch information
Kent C. Dodds authored and kentcdodds committed Mar 12, 2020
1 parent 13b00ea commit 4c0193c
Show file tree
Hide file tree
Showing 20 changed files with 286 additions and 190 deletions.
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@
"@types/testing-library__dom": "^6.12.1",
"aria-query": "^4.0.2",
"dom-accessibility-api": "^0.3.0",
"pretty-format": "^25.1.0",
"wait-for-expect": "^3.0.2"
"pretty-format": "^25.1.0"
},
"devDependencies": {
"@testing-library/jest-dom": "^5.1.1",
Expand All @@ -61,7 +60,8 @@
"rules": {
"import/prefer-default-export": "off",
"import/no-unassigned-import": "off",
"import/no-useless-path-segments": "off"
"import/no-useless-path-segments": "off",
"no-console": "off"
}
},
"eslintIgnore": [
Expand Down
52 changes: 52 additions & 0 deletions src/__tests__/fake-timers.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ jest.useFakeTimers()
jest.resetModules()

const {
wait,
waitForElement,
waitForDomChange,
waitForElementToBeRemoved,
Expand All @@ -42,6 +43,15 @@ test('waitForElementToBeRemoved: times out after 4500ms by default', () => {
return promise
})

test('wait: can time out', async () => {
const promise = wait(() => {
// eslint-disable-next-line no-throw-literal
throw undefined
})
jest.advanceTimersByTime(4600)
await expect(promise).rejects.toThrow(/timed out/i)
})

test('waitForElement: can time out', async () => {
const promise = waitForElement(() => {})
jest.advanceTimersByTime(4600)
Expand Down Expand Up @@ -85,3 +95,45 @@ test('waitForDomChange: can specify our own timeout time', async () => {
// timed out
await expect(promise).rejects.toThrow(/timed out/i)
})

test('wait: ensures the interval is greater than 0', async () => {
// Arrange
const spy = jest.fn()
spy.mockImplementationOnce(() => {
throw new Error('first time does not work')
})
const promise = wait(spy, {interval: 0})
expect(spy).toHaveBeenCalledTimes(1)
spy.mockClear()

// Act
// this line will throw an error if wait does not make the interval 1 instead of 0
// which is why it does that!
jest.advanceTimersByTime(0)

// Assert
expect(spy).toHaveBeenCalledTimes(0)
spy.mockImplementationOnce(() => 'second time does work')

// Act
jest.advanceTimersByTime(1)
await promise

// Assert
expect(spy).toHaveBeenCalledTimes(1)
})

test('wait: times out if it runs out of attempts', () => {
const spy = jest.fn(() => {
throw new Error('example error')
})
// there's a bug with this rule here...
// eslint-disable-next-line jest/valid-expect
const promise = expect(
wait(spy, {interval: 1, timeout: 3}),
).rejects.toThrowErrorMatchingInlineSnapshot(`"example error"`)
jest.advanceTimersByTime(1)
jest.advanceTimersByTime(1)
jest.advanceTimersByTime(1)
return promise
})
2 changes: 0 additions & 2 deletions src/__tests__/pretty-dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,5 +77,3 @@ describe('prettyDOM fails with first parameter without outerHTML field', () => {
)
})
})

/* eslint no-console:0 */
2 changes: 0 additions & 2 deletions src/__tests__/role-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,5 +184,3 @@ test.each([

expect(isInaccessible(container.querySelector('button'))).toBe(expected)
})

/* eslint no-console:0 */
2 changes: 0 additions & 2 deletions src/__tests__/screen.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,5 +61,3 @@ test('exposes debug method', () => {
`)
console.log.mockClear()
})

/* eslint no-console:0 */
8 changes: 8 additions & 0 deletions src/__tests__/wait-for-dom-change.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ beforeEach(() => {
jest.useRealTimers()
jest.resetModules()
waitForDomChange = importModule()
console.warn.mockClear()
})

test('waits for the dom to change in the document', async () => {
Expand All @@ -34,6 +35,13 @@ test('waits for the dom to change in the document', async () => {
},
]
`)
expect(console.warn.mock.calls).toMatchInlineSnapshot(`
Array [
Array [
"\`waitForDomChange\` has been deprecated. Use \`wait\` instead: https://testing-library.com/docs/dom-testing-library/api-async#waitfor.",
],
]
`)
})

test('waits for the dom to change in a specified container', async () => {
Expand Down
21 changes: 18 additions & 3 deletions src/__tests__/wait-for-element-to-be-removed.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,23 +49,23 @@ test('requires a function as the first parameter', () => {
return expect(
waitForElementToBeRemoved(),
).rejects.toThrowErrorMatchingInlineSnapshot(
`"waitForElementToBeRemoved requires a function as the first parameter"`,
`"waitForElementToBeRemoved requires a callback as the first parameter"`,
)
})

test('requires an element to exist first', () => {
return expect(
waitForElementToBeRemoved(() => null),
).rejects.toThrowErrorMatchingInlineSnapshot(
`"The callback function which was passed did not return an element or non-empty array of elements. waitForElementToBeRemoved requires that the element(s) exist before waiting for removal."`,
`"The callback function which was passed did not return an element or non-empty array of elements. waitForElementToBeRemoved requires that the element(s) exist(s) before waiting for removal."`,
)
})

test('requires an unempty array of elements to exist first', () => {
return expect(
waitForElementToBeRemoved(() => []),
).rejects.toThrowErrorMatchingInlineSnapshot(
`"The callback function which was passed did not return an element or non-empty array of elements. waitForElementToBeRemoved requires that the element(s) exist before waiting for removal."`,
`"The callback function which was passed did not return an element or non-empty array of elements. waitForElementToBeRemoved requires that the element(s) exist(s) before waiting for removal."`,
)
})

Expand Down Expand Up @@ -117,3 +117,18 @@ test("doesn't change jest's timers value when importing the module", () => {

expect(window.setTimeout._isMockFunction).toEqual(true)
})

test('rethrows non-testing-lib errors', () => {
let throwIt = false
const div = document.createElement('div')
const error = new Error('my own error')
return expect(
waitForElementToBeRemoved(() => {
if (throwIt) {
throw error
}
throwIt = true
return div
}),
).rejects.toBe(error)
})
8 changes: 8 additions & 0 deletions src/__tests__/wait-for-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ beforeEach(() => {
jest.useRealTimers()
jest.resetModules()
waitForElement = importModule()
console.warn.mockClear()
})

test('waits for element to appear in the document', async () => {
Expand All @@ -18,6 +19,13 @@ test('waits for element to appear in the document', async () => {
setTimeout(() => rerender('<div data-testid="div" />'))
const element = await promise
expect(element).toBeInTheDocument()
expect(console.warn.mock.calls).toMatchInlineSnapshot(`
Array [
Array [
"\`waitForElement\` has been deprecated. Use a \`find*\` query (preferred: https://testing-library.com/docs/dom-testing-library/api-queries#findby) or use \`wait\` instead (it's the same API, so you can find/replace): https://testing-library.com/docs/dom-testing-library/api-async#waitfor",
],
]
`)
})

test('waits for element to appear in a specified container', async () => {
Expand Down
23 changes: 23 additions & 0 deletions src/__tests__/wait-for.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import {waitFor} from '../'

test('waits callback to not throw an error', async () => {
const spy = jest.fn()
// we are using random timeout here to simulate a real-time example
// of an async operation calling a callback at a non-deterministic time
const randomTimeout = Math.floor(Math.random() * 60)
setTimeout(spy, randomTimeout)

await waitFor(() => expect(spy).toHaveBeenCalledTimes(1))
expect(spy).toHaveBeenCalledWith()
})

test('can timeout after the given timeout time', async () => {
const error = new Error('throws every time')
const result = await waitFor(
() => {
throw error
},
{timeout: 8, interval: 5},
).catch(e => e)
expect(result).toBe(error)
})
19 changes: 0 additions & 19 deletions src/__tests__/wait.js

This file was deleted.

6 changes: 4 additions & 2 deletions src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {prettyDOM} from './pretty-dom'
// './queries' are query functions.
let config = {
testIdAttribute: 'data-testid',
asyncUtilTimeout: 4500,
asyncUtilTimeout: 1000,
// this is to support React's async `act` function.
// forcing react-testing-library to wrap all async functions would've been
// a total nightmare (consider wrapping every findBy* query and then also
Expand All @@ -19,9 +19,11 @@ let config = {

// called when getBy* queries fail. (message, container) => Error
getElementError(message, container) {
return new Error(
const error = new Error(
[message, prettyDOM(container)].filter(Boolean).join('\n\n'),
)
error.name = 'TestingLibraryElementError'
return error
},
}

Expand Down
2 changes: 1 addition & 1 deletion src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as queries from './queries'
import * as queryHelpers from './query-helpers'

export * from './queries'
export * from './wait'
export * from './wait-for'
export * from './wait-for-element'
export * from './wait-for-element-to-be-removed'
export * from './wait-for-dom-change'
Expand Down
4 changes: 1 addition & 3 deletions src/pretty-dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const inNode = () =>
const getMaxLength = dom =>
inCypress(dom)
? 0
: typeof process !== 'undefined' && process.env.DEBUG_PRINT_LIMIT || 7000
: (typeof process !== 'undefined' && process.env.DEBUG_PRINT_LIMIT) || 7000

const {DOMElement, DOMCollection} = prettyFormat.plugins

Expand Down Expand Up @@ -64,5 +64,3 @@ function prettyDOM(dom, maxLength, options) {
const logDOM = (...args) => console.log(prettyDOM(...args))

export {prettyDOM, logDOM}

/* eslint no-console:0 */
2 changes: 0 additions & 2 deletions src/role-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,5 +176,3 @@ export {
prettyRoles,
isInaccessible,
}

/* eslint no-console:0 */
11 changes: 11 additions & 0 deletions src/wait-for-dom-change.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ import {
} from './helpers'
import {getConfig} from './config'

let hasWarned = false

// deprecated... TODO: remove this method. People should use wait instead
// the reasoning is that waiting for just any DOM change is an implementation
// detail. People should be waiting for a specific thing to change.
function waitForDomChange({
container = getDocument(),
timeout = getConfig().asyncUtilTimeout,
Expand All @@ -18,6 +23,12 @@ function waitForDomChange({
characterData: true,
},
} = {}) {
if (!hasWarned) {
hasWarned = true
console.warn(
`\`waitForDomChange\` has been deprecated. Use \`wait\` instead: https://testing-library.com/docs/dom-testing-library/api-async#waitfor.`,
)
}
return new Promise((resolve, reject) => {
const timer = setTimeout(onTimeout, timeout)
const observer = newMutationObserver(onMutation)
Expand Down
Loading

0 comments on commit 4c0193c

Please sign in to comment.