Skip to content

Commit 092d52f

Browse files
crutchcorncrutchcorn
committed
Add in better error handling for instance problems
1 parent c8bb37d commit 092d52f

File tree

7 files changed

+87
-66
lines changed

7 files changed

+87
-66
lines changed

‎src/__tests__/wait-for.js‎

Lines changed: 34 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {waitFor} from '../'
2-
// import {configure, getConfig} from '../config'
2+
import {configure, getConfig} from '../config'
33
// import {render} from '../pure'
44

55
function deferred() {
@@ -102,15 +102,12 @@ test('provides an improved stack trace if the thrown error is a TestingLibraryEl
102102
expect(result.stack).not.toBe(originalStackTrace)
103103
})
104104

105-
// test('throws nice error if provided callback is not a function', () => {
106-
// const {queryByTestId} = renderIntoDocument(`
107-
// <div data-testid="div"></div>
108-
// `)
109-
// const someElement = queryByTestId('div')
110-
// expect(() => waitFor(someElement)).toThrow(
111-
// 'Received `callback` arg must be a function',
112-
// )
113-
// })
105+
test('throws nice error if provided callback is not a function', () => {
106+
const someElement = 'Hello'
107+
expect(() => waitFor(someElement)).toThrow(
108+
'Received `callback` arg must be a function',
109+
)
110+
})
114111

115112
// test('timeout logs a pretty DOM', async () => {
116113
// renderIntoDocument(`<div id="pretty">how pretty</div>`)
@@ -137,24 +134,23 @@ test('provides an improved stack trace if the thrown error is a TestingLibraryEl
137134
// `)
138135
// })
139136

140-
// test('should delegate to config.getElementError', async () => {
141-
// const originalConfig = getConfig()
142-
// const elementError = new Error('Custom element error')
143-
// const getElementError = jest.fn().mockImplementation(() => elementError)
144-
// configure({getElementError})
145-
//
146-
// renderIntoDocument(`<div id="pretty">how pretty</div>`)
147-
// const error = await waitFor(
148-
// () => {
149-
// throw new Error('always throws')
150-
// },
151-
// {timeout: 1},
152-
// ).catch(e => e)
153-
//
154-
// expect(getElementError).toBeCalledTimes(1)
155-
// expect(error.message).toMatchInlineSnapshot(`Custom element error`)
156-
// configure(originalConfig)
157-
// })
137+
test('should delegate to config.getInstanceError', async () => {
138+
const originalConfig = getConfig()
139+
const elementError = new Error('Custom instance error')
140+
const getInstanceError = jest.fn().mockImplementation(() => elementError)
141+
configure({getInstanceError})
142+
143+
const error = await waitFor(
144+
() => {
145+
throw new Error('always throws')
146+
},
147+
{timeout: 1},
148+
).catch(e => e)
149+
150+
expect(getInstanceError).toBeCalledTimes(1)
151+
expect(error.message).toMatchInlineSnapshot(`Custom instance error`)
152+
configure(originalConfig)
153+
})
158154

159155
test('when a promise is returned, it does not call the callback again until that promise rejects', async () => {
160156
const sleep = t => new Promise(r => setTimeout(r, t))
@@ -178,22 +174,16 @@ test('when a promise is returned, it does not call the callback again until that
178174
await waitForPromise
179175
})
180176

181-
// test('when a promise is returned, if that is not resolved within the timeout, then waitFor is rejected', async () => {
182-
// const sleep = t => new Promise(r => setTimeout(r, t))
183-
// const {promise} = deferred()
184-
// const waitForError = waitFor(() => promise, {timeout: 1}).catch(e => e)
185-
// await sleep(5)
186-
//
187-
// expect((await waitForError).message).toMatchInlineSnapshot(`
188-
// Timed out in waitFor.
189-
//
190-
// Ignored nodes: comments, <script />, <style />
191-
// <html>
192-
// <head />
193-
// <body />
194-
// </html>
195-
// `)
196-
// })
177+
test('when a promise is returned, if that is not resolved within the timeout, then waitFor is rejected', async () => {
178+
const sleep = t => new Promise(r => setTimeout(r, t))
179+
const {promise} = deferred()
180+
const waitForError = waitFor(() => promise, {timeout: 1}).catch(e => e)
181+
await sleep(5)
182+
183+
expect((await waitForError).message).toMatchInlineSnapshot(
184+
`Timed out in waitFor.`,
185+
)
186+
})
197187

198188
test('if you switch from fake timers to real timers during the wait period you get an error', async () => {
199189
jest.useFakeTimers()

‎src/config.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,11 @@ let config: InternalConfig = {
3131
throwSuggestions: false,
3232

3333
// called when getBy* queries fail. (message, container) => Error
34-
getElementError(message, testInstance) {
34+
getInstanceError(message, testInstance) {
3535
const error = new Error(
3636
[
3737
message,
38-
`\n${testInstance.stdoutStr}`,
38+
testInstance ? `\n${testInstance.stdoutStr}` : '',
3939
]
4040
.filter(Boolean)
4141
.join('\n\n'),

‎src/helpers.js‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,33 @@ function jestFakeTimersAreEnabled() {
1212
return false
1313
}
1414

15-
export {jestFakeTimersAreEnabled}
15+
const instance = {current: undefined};
16+
17+
afterEach(() => {
18+
instance.current = undefined;
19+
})
20+
21+
function getCurrentInstance() {
22+
/**
23+
* Worth mentioning that this deviates from the upstream implementation
24+
* of `dom-testing-library`'s `getDocument`, which throws an error whenever
25+
* `window` is not defined.
26+
*
27+
* Admittedly, this is another way that `cli-testing-library` will need to figure out
28+
* the right solution to this problem, since there is no omni-present parent `instance`
29+
* in a CLI like there is in a browser. (although FWIW, "process" might work)
30+
*
31+
* Have ideas how to solve? Please let us know:
32+
* https://github.com/crutchcorn/cli-testing-library/issues/2
33+
*/
34+
return instance.current
35+
}
36+
37+
// TODO: Does this need to be namespaced for each test that runs?
38+
// That way, we don't end up with a "singleton" that ends up wiped between
39+
// parallel tests.
40+
function setCurrentInstance(newInstance) {
41+
instance.current = newInstance;
42+
}
43+
44+
export {jestFakeTimersAreEnabled, setCurrentInstance, getCurrentInstance}

‎src/pure.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import stripFinalNewline from 'strip-final-newline'
99
import {RenderOptions, TestInstance} from '../types/pure'
1010
import {_runObservers} from './mutation-observer'
1111
import {getQueriesForElement} from './get-queries-for-instance'
12+
import {setCurrentInstance} from "./helpers";
1213

1314
async function render(
1415
command: string,
@@ -71,6 +72,8 @@ async function render(
7172

7273
await execOutputAPI._isReady
7374

75+
setCurrentInstance(execOutputAPI);
76+
7477
return Object.assign(
7578
execOutputAPI,
7679
{

‎src/query-helpers.ts‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,15 @@ import {waitFor} from './wait-for'
1212
import {TestInstance} from "../types/pure";
1313
import {getConfig} from "./config";
1414

15-
function getElementError(message: string | null, instance: TestInstance) {
16-
return getConfig().getElementError(message, instance)
15+
function getInstanceError(message: string | null, instance: TestInstance) {
16+
return getConfig().getInstanceError(message, instance)
1717
}
1818

1919
function getSuggestionError(
2020
suggestion: {toString(): string},
2121
container: TestInstance,
2222
) {
23-
return getConfig().getElementError(
23+
return getConfig().getInstanceError(
2424
`A better query is available, try this:
2525
${suggestion.toString()}
2626
`,
@@ -31,15 +31,15 @@ ${suggestion.toString()}
3131
// this accepts a query function and returns a function which throws an error
3232
// if an empty list of elements is returned
3333
function makeGetQuery<Arguments extends unknown[]>(
34-
queryBy: (container: TestInstance, ...args: Arguments) => TestInstance | null,
34+
queryBy: (instance: TestInstance, ...args: Arguments) => TestInstance | null,
3535
getMissingError: GetErrorFunction<Arguments>,
3636
) {
37-
return (container: TestInstance, ...args: Arguments) => {
38-
const el = queryBy(container, ...args)
37+
return (instance: TestInstance, ...args: Arguments) => {
38+
const el = queryBy(instance, ...args)
3939
if (!el) {
40-
throw getConfig().getElementError(
41-
getMissingError(container, ...args),
42-
container,
40+
throw getConfig().getInstanceError(
41+
getMissingError(instance, ...args),
42+
instance,
4343
)
4444
}
4545

@@ -134,7 +134,7 @@ function buildQueries(
134134
}
135135

136136
export {
137-
getElementError,
137+
getInstanceError,
138138
wrapSingleQueryWithSuggestion,
139139
makeFindQuery,
140140
buildQueries,

‎src/wait-for.js‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
// Migrated from: https://github.com/testing-library/dom-testing-library/blob/main/src/wait-for.js
22
// TODO: Migrate back to use `config.js` file
3-
import {jestFakeTimersAreEnabled} from './helpers'
3+
import {getCurrentInstance, jestFakeTimersAreEnabled} from './helpers'
44
import {MutationObserver} from './mutation-observer'
55
import {getConfig} from "./config";
6-
// import {getConfig, runWithExpensiveErrorDiagnosticsDisabled} from './config'
76

87
// This is so the stack trace the developer sees is one that's
98
// closer to their code (because async stack traces are hard to follow).
@@ -14,15 +13,16 @@ function copyStackTrace(target, source) {
1413
function waitFor(
1514
callback,
1615
{
16+
instance = getCurrentInstance(),
1717
timeout = getConfig().asyncUtilTimeout,
1818
showOriginalStackTrace = getConfig().showOriginalStackTrace,
1919
stackTraceError,
2020
interval = 50,
2121
onTimeout = error => {
22-
/*error.message = getConfig().getElementError(
22+
error.message = getConfig().getInstanceError(
2323
error.message,
24-
container,
25-
).message*/
24+
instance,
25+
).message
2626
return error
2727
},
2828
},
@@ -169,10 +169,9 @@ function waitForWrapper(callback, options) {
169169
// create the error here so its stack trace is as close to the
170170
// calling code as possible
171171
const stackTraceError = new Error('STACK_TRACE_MESSAGE')
172-
return waitFor(callback, {stackTraceError, ...options})
173-
/*getConfig().asyncWrapper(() =>
172+
return getConfig().asyncWrapper(() =>
174173
waitFor(callback, {stackTraceError, ...options}),
175-
)*/
174+
)
176175
}
177176

178177
export {waitForWrapper as waitFor}

‎types/config.d.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ export interface Config {
1616
defaultHidden: boolean
1717
showOriginalStackTrace: boolean
1818
throwSuggestions: boolean
19-
getElementError: (message: string | null, container: TestInstance) => Error
19+
getInstanceError: (message: string | null, container: TestInstance) => Error
2020
}
2121

2222
export interface ConfigFn {

0 commit comments

Comments
 (0)