From 07c225c44f31262c5f482f5a6f17e6d748e687bf Mon Sep 17 00:00:00 2001 From: jgjgill <79239852+jgjgill@users.noreply.github.com> Date: Sun, 28 Jul 2024 20:32:40 +0900 Subject: [PATCH] test(react): improve `ErrorBoundaryGroup` test code (#1157) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Overview Hi, I've been looking at the `ErrorBoundaryGroup` test code and trying to improve it. - I changed `Throw.reset()` to be managed by `beforeEach`. - `useErrorBoundaryGroup` related tests only have tests that fail. I add success case. **Question** [Related Code](https://github.com/toss/suspensive/blob/cbbb6ffe649cfd4442da5fcf55e92d0afdf0fb3c/packages/utils/src/test-utils/index.tsx#L4-L26) The `current` value of `isNeedThrowGlobal` used by the `Throw` object doesn't seem to change. There doesn't seem to be anything currently happening that would cause `isNeedThrowGlobal.current = true`. 스크린샷 2024-07-28 오후 6 54 04 So the absence of `Throw.reset()` in the current test doesn't seem to change the result of the test. Was this code added for safety? Thanks. 🙇‍♂️ ## PR Checklist - [x] I did below actions if need 1. I read the [Contributing Guide](https://github.com/toss/suspensive/blob/main/CONTRIBUTING.md) 2. I added documents and tests. Co-authored-by: Jonghyeon Ko --- .../react/src/ErrorBoundaryGroup.spec.tsx | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/packages/react/src/ErrorBoundaryGroup.spec.tsx b/packages/react/src/ErrorBoundaryGroup.spec.tsx index eb0a91b0b..9d0e8c015 100644 --- a/packages/react/src/ErrorBoundaryGroup.spec.tsx +++ b/packages/react/src/ErrorBoundaryGroup.spec.tsx @@ -13,6 +13,8 @@ const innerErrorBoundaryCount = 3 const resetButtonText = 'reset button' describe('', () => { + beforeEach(() => Throw.reset()) + it('should reset all ErrorBoundaries in children', async () => { render( @@ -31,7 +33,6 @@ describe('', () => { expect(screen.getAllByText(TEXT).length).toBe(innerErrorBoundaryCount) await waitFor(() => expect(screen.getAllByText(ERROR_MESSAGE).length).toBe(innerErrorBoundaryCount)) - Throw.reset() fireEvent.click(screen.getByRole('button', { name: resetButtonText })) expect(screen.getAllByText(TEXT).length).toBe(innerErrorBoundaryCount) @@ -58,7 +59,6 @@ describe('', () => { expect(screen.getAllByText(TEXT).length).toBe(innerErrorBoundaryCount) await waitFor(() => expect(screen.getAllByText(ERROR_MESSAGE).length).toBe(innerErrorBoundaryCount)) - Throw.reset() fireEvent.click(screen.getByRole('button', { name: resetButtonText })) expect(screen.getAllByText(TEXT).length).toBe(innerErrorBoundaryCount - 1) @@ -67,7 +67,21 @@ describe('', () => { }) describe('useErrorBoundaryGroup', () => { - it('should throw error without ErrorBoundaryGroup in parent', () => { + it('should guarantee hook calling position is in children of ErrorBoundaryGroup', () => { + expect( + render( + + {createElement(() => { + useErrorBoundaryGroup() + return <> + })} + + <>{TEXT} + + + ).getByText(TEXT) + ).toBeInTheDocument() + expect(() => render( createElement(() => { @@ -76,6 +90,7 @@ describe('useErrorBoundaryGroup', () => { }) ) ).toThrow(Message_useErrorBoundaryGroup_this_hook_should_be_called_in_ErrorBoundary_props_children) + try { render( createElement(() => {