feat: Add test coverage for executeSafePromise error handling paths
#3311
Conversation
|
|
Co-authored-by: mondoreale <[email protected]>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
| describe('environment detection', () => { | ||
| it('should detect Node environment when process.exit is defined', async () => { | ||
| const mockExit = jest.fn() as any | ||
| const originalProcessExit = process.exit | ||
| process.exit = mockExit | ||
|
|
||
| await executeSafePromise(async () => { | ||
| throw new Error('Test') | ||
| }) | ||
|
|
||
| expect(mockExit).toHaveBeenCalled() | ||
| // eslint-disable-next-line require-atomic-updates | ||
| process.exit = originalProcessExit | ||
| }) | ||
|
|
||
| it('should detect browser environment when process.exit is undefined', async () => { | ||
| const originalProcess = global.process | ||
| // @ts-expect-error - intentionally setting to undefined for test | ||
| delete (global as any).process.exit | ||
|
|
||
| await expect(executeSafePromise(async () => { | ||
| throw new Error('Test') | ||
| })).rejects.toThrow('executeSafePromise: Assertion failure!') | ||
|
|
||
| // eslint-disable-next-line require-atomic-updates | ||
| global.process = originalProcess | ||
| }) | ||
| }) |
There was a problem hiding this comment.
@copilot isn't this block redundant? Envs are already part of the earlier tests.
There was a problem hiding this comment.
You're right, that block was redundant. The environment detection is already tested by the Node/Electron and browser environment test blocks. Removed in ff0e81f.
Co-authored-by: mondoreale <[email protected]>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
mondoreale
changed the title
executeSafePromise error handling paths
mondoreale
merged commit 245d1c8
into
cross-environment-executeSafePromise
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| afterEach(() => { | ||
| // Restore process | ||
| global.process = originalProcess | ||
| }) |
There was a problem hiding this comment.
Global process.exit remains deleted after browser tests
In the browser environment test block, process.exit is deleted from the global process object. Since originalProcess is a reference to the same object, assigning it back in afterEach fails to restore the deleted property. This leaves process.exit as undefined for subsequent tests, potentially breaking other parts of the test suite.
Summary
Adds missing test coverage for error handling paths in
executeSafePromiseintroduced by the browser compatibility refactor (#3310).Changes
process.exit(1)is called on promise rejection and fatal errors are loggedcausepropertyThe environment detection behavior is implicitly verified by the Node and browser test blocks, which mock or remove
process.exitand verify the appropriate code path is taken.Checklist before requesting a review
TODOcomments left behind are meant to be left in.✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.
Note
Expands test coverage for
executeSafePromiseto validate error-handling in different runtimes.packages/utils/test/executeSafePromise.test.tsto assertprocess.exit(1)is invoked on rejection and that exit is triggered after fatal errorsprocess.exitto assert a thrown error with messageexecuteSafePromise: Assertion failure!and propercausechainingWritten by Cursor Bugbot for commit ff0e81f. This will update automatically on new commits. Configure here.