Skip to content

Commit 02af88f

Browse files
committed
fix: implement better error handling
1 parent b5790cc commit 02af88f

File tree

2 files changed

+82
-47
lines changed

2 files changed

+82
-47
lines changed

src/helpers.js

Lines changed: 43 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
11
import {fireEvent} from "./events";
22

33
function jestFakeTimersAreEnabled() {
4-
/* istanbul ignore else */
5-
if (typeof jest !== 'undefined' && jest !== null) {
6-
return (
7-
// legacy timers
8-
setTimeout._isMockFunction === true ||
9-
// modern timers
10-
Object.prototype.hasOwnProperty.call(setTimeout, 'clock')
11-
)
12-
}
13-
// istanbul ignore next
14-
return false
4+
/* istanbul ignore else */
5+
if (typeof jest !== 'undefined' && jest !== null) {
6+
return (
7+
// legacy timers
8+
setTimeout._isMockFunction === true ||
9+
// modern timers
10+
Object.prototype.hasOwnProperty.call(setTimeout, 'clock')
11+
)
12+
}
13+
// istanbul ignore next
14+
return false
1515
}
1616

1717
const instance = {current: undefined};
@@ -21,35 +21,46 @@ const instance = {current: undefined};
2121
* @see https://testing-library.com/docs/react-testing-library/setup/#skipping-auto-cleanup
2222
*/
2323
afterEach(() => {
24-
/**
25-
* Behavior should be customizability like it is with `react-testing-library`
26-
* @see https://testing-library.com/docs/react-testing-library/setup/#skipping-auto-cleanup
27-
*/
28-
if (instance.current) fireEvent.sigkill(instance.current);
29-
instance.current = undefined;
24+
/**
25+
* Behavior should be customizability like it is with `react-testing-library`
26+
* @see https://testing-library.com/docs/react-testing-library/setup/#skipping-auto-cleanup
27+
*/
28+
if (instance.current) fireEvent.sigkill(instance.current);
29+
instance.current = undefined;
3030
})
3131

3232
function getCurrentInstance() {
33-
/**
34-
* Worth mentioning that this deviates from the upstream implementation
35-
* of `dom-testing-library`'s `getDocument`, which throws an error whenever
36-
* `window` is not defined.
37-
*
38-
* Admittedly, this is another way that `cli-testing-library` will need to figure out
39-
* the right solution to this problem, since there is no omni-present parent `instance`
40-
* in a CLI like there is in a browser. (although FWIW, "process" might work)
41-
*
42-
* Have ideas how to solve? Please let us know:
43-
* https://github.com/crutchcorn/cli-testing-library/issues/2
44-
*/
45-
return instance.current
33+
/**
34+
* Worth mentioning that this deviates from the upstream implementation
35+
* of `dom-testing-library`'s `getDocument`, which throws an error whenever
36+
* `window` is not defined.
37+
*
38+
* Admittedly, this is another way that `cli-testing-library` will need to figure out
39+
* the right solution to this problem, since there is no omni-present parent `instance`
40+
* in a CLI like there is in a browser. (although FWIW, "process" might work)
41+
*
42+
* Have ideas how to solve? Please let us know:
43+
* https://github.com/crutchcorn/cli-testing-library/issues/2
44+
*/
45+
return instance.current
4646
}
4747

4848
// TODO: Does this need to be namespaced for each test that runs?
4949
// That way, we don't end up with a "singleton" that ends up wiped between
5050
// parallel tests.
5151
function setCurrentInstance(newInstance) {
52-
instance.current = newInstance;
52+
instance.current = newInstance;
5353
}
5454

55-
export {jestFakeTimersAreEnabled, setCurrentInstance, getCurrentInstance}
55+
function debounce(func, timeout) {
56+
let timer;
57+
return (...args) => {
58+
clearTimeout(timer);
59+
timer = setTimeout(() => {
60+
// eslint-disable-next-line @babel/no-invalid-this
61+
func.apply(this, args);
62+
}, timeout);
63+
};
64+
}
65+
66+
export {jestFakeTimersAreEnabled, setCurrentInstance, getCurrentInstance, debounce}

src/pure.ts

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ import {RenderOptions, RenderResult, TestInstance} from '../types/pure'
1010
import {_runObservers} from './mutation-observer'
1111
import {getQueriesForElement} from './get-queries-for-instance'
1212
import {getFireEventForElement} from './events'
13-
import {setCurrentInstance} from "./helpers";
13+
import {debounce, setCurrentInstance} from "./helpers";
14+
import {getConfig} from "./config";
1415

1516
async function render(
1617
command: string,
@@ -47,39 +48,62 @@ async function render(
4748
set stdoutStr(val: string) {},
4849
}
4950

51+
/**
52+
* This method does not throw errors after-the-fact,
53+
* meaning that if post-render errors are thrown,
54+
* they will be missed
55+
*
56+
* THINK: Should we should simply `throw` when this occurs?
57+
* What about cleanup?
58+
* What about interactive errors?
59+
*/
60+
let _errorHasOccured = false;
61+
const _errors: Array<string | Buffer | Error> = [];
62+
5063
exec.stdout.on('data', (result: string | Buffer) => {
5164
const resStr = stripFinalNewline(result as string);
5265
execOutputAPI.stdoutArr.push(resStr)
5366
_runObservers()
54-
if (_readyPromiseInternals && !_isReadyResolved) {
67+
if (!_errorHasOccured && _readyPromiseInternals && !_isReadyResolved) {
5568
_readyPromiseInternals.resolve()
5669
_isReadyResolved = true;
5770
}
71+
// stdout might contain relevant error info
72+
if (_errorHasOccured && _readyPromiseInternals && !_isReadyResolved) {
73+
_errors.push(result);
74+
}
5875
})
5976

77+
const _onError = () => {
78+
if (!_readyPromiseInternals) return;
79+
_readyPromiseInternals.reject(new Error(_errors.join('\n')))
80+
_isReadyResolved = true;
81+
}
82+
83+
const config = getConfig()
84+
// TODO: Migrate to new config option?
85+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-unsafe-call
86+
const _throttledOnError = debounce(_onError, config.asyncUtilTimeout);
87+
6088
exec.stdout.on('error', result => {
6189
if (_readyPromiseInternals && !_isReadyResolved) {
62-
_readyPromiseInternals.reject(result)
63-
_isReadyResolved = true;
90+
_errors.push(result);
91+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-unsafe-call
92+
_throttledOnError();
93+
_errorHasOccured = true;
6494
}
6595
})
6696

6797
exec.stderr.on('data', (result: string) => {
6898
if (_readyPromiseInternals && !_isReadyResolved) {
69-
/**
70-
* TODO: We're getting an error where "result" is only the first line of many drawn.
71-
* Let's go ahead and set a timeout var in `config.js` and debounce the rejection
72-
*
73-
* Then, we'll set a boolean to not do any other kind of logging to stdout (or promise resolve)
74-
* until that timeout/debounce has expired.
75-
*
76-
* Also merge with the `'error'` field above
77-
*/
78-
_readyPromiseInternals.reject(new Error(result))
79-
_isReadyResolved = true;
99+
_errors.push(result);
100+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-unsafe-call
101+
_throttledOnError();
102+
_errorHasOccured = true;
80103
}
81104
})
82105

106+
// TODO: Replace with `debug()` function
83107
if (opts.debug) {
84108
exec.stdout.pipe(process.stdout)
85109
}

0 commit comments

Comments
 (0)