Skip to content

Commit

Permalink
test(react): improve ErrorBoundaryGroup test code (#1157)
Browse files Browse the repository at this point in the history
# 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`.

<img width="1142" alt="스크린샷 2024-07-28 오후 6 54 04"
src="https://github.com/user-attachments/assets/7d3b874a-2407-4684-98f6-7c6638b8d79a">

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. 🙇‍♂️

<!--
    A clear and concise description of what this pr is about.
 -->

## 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 <[email protected]>
  • Loading branch information
jgjgill and manudeli authored Jul 28, 2024
1 parent a9bb395 commit 07c225c
Showing 1 changed file with 18 additions and 3 deletions.
21 changes: 18 additions & 3 deletions packages/react/src/ErrorBoundaryGroup.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ const innerErrorBoundaryCount = 3
const resetButtonText = 'reset button'

describe('<ErrorBoundaryGroup/>', () => {
beforeEach(() => Throw.reset())

it('should reset all ErrorBoundaries in children', async () => {
render(
<ErrorBoundaryGroup>
Expand All @@ -31,7 +33,6 @@ describe('<ErrorBoundaryGroup/>', () => {

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)
Expand All @@ -58,7 +59,6 @@ describe('<ErrorBoundaryGroup/>', () => {

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)
Expand All @@ -67,7 +67,21 @@ describe('<ErrorBoundaryGroup/>', () => {
})

describe('useErrorBoundaryGroup', () => {
it('should throw error without ErrorBoundaryGroup in parent', () => {
it('should guarantee hook calling position is in children of ErrorBoundaryGroup', () => {
expect(
render(
<ErrorBoundaryGroup>
{createElement(() => {
useErrorBoundaryGroup()
return <></>
})}
<ErrorBoundary fallback={ERROR_MESSAGE}>
<>{TEXT}</>
</ErrorBoundary>
</ErrorBoundaryGroup>
).getByText(TEXT)
).toBeInTheDocument()

expect(() =>
render(
createElement(() => {
Expand All @@ -76,6 +90,7 @@ describe('useErrorBoundaryGroup', () => {
})
)
).toThrow(Message_useErrorBoundaryGroup_this_hook_should_be_called_in_ErrorBoundary_props_children)

try {
render(
createElement(() => {
Expand Down

0 comments on commit 07c225c

Please sign in to comment.