test(contact-point): simplifies container test#1400
Conversation
WalkthroughThis update exports the Changes
Sequence Diagram(s)sequenceDiagram
participant TestSuite
participant Container
participant ChildCapturer
TestSuite->>ChildCapturer: createContainerTestingChildCapturer()
TestSuite->>Container: render with ChildCapturer.renderChild as child
Container->>ChildCapturer: invoke renderChild(childProps)
TestSuite->>ChildCapturer: awaitChild()
ChildCapturer-->>TestSuite: childProps
TestSuite->>childProps: call method (e.g., create, onRemove)
Container->>TestSuite: update state/props as a result
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1400 +/- ##
==========================================
+ Coverage 36.49% 36.50% +0.01%
==========================================
Files 818 818
Lines 19678 19678
Branches 3630 3630
==========================================
+ Hits 7181 7183 +2
+ Misses 12172 12170 -2
Partials 325 325
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
ygrishajev
force-pushed
the
feature/contact-point
branch
from
ebb7478 to
a9b9e43
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/deploy-web/tests/unit/container-testing-child-capturer.tsx (2)
3-8: Consider using arrow functions to eliminate constructor binding.The manual binding in the constructor can be simplified by using arrow functions, which automatically bind
thiscontext.-class ContainerTestingChildCapturer<T> { - private child?: T; - - constructor() { - this.renderChild = this.renderChild.bind(this); - } +class ContainerTestingChildCapturer<T> { + private child?: T;And update the method to an arrow function:
- renderChild(props: T) { + renderChild = (props: T) => { this.child = props; return null; - } + };
15-22: Improve error message specificity.The current error message "Child not rendered yet" doesn't distinguish between the child not being set vs the condition not being met, making debugging more difficult.
async awaitChild(condition: (child: T) => boolean = () => true): Promise<T> { return await waitFor(() => { - if (!this.child || !condition(this.child)) { - throw new Error("Child not rendered yet"); + if (!this.child) { + throw new Error("Child component has not been rendered yet"); + } + if (!condition(this.child)) { + throw new Error("Child component condition not met"); } return this.child as T; }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/api/env/.env(0 hunks)apps/deploy-web/src/components/alerts/ContactPointCreateContainer/ContactPointCreateContainer.tsx(1 hunks)apps/deploy-web/src/components/alerts/ContactPointCreateContainer/ContactPointFormContainer.spec.tsx(4 hunks)apps/deploy-web/src/components/alerts/ContactPointsListContainer/ContactPointsListContainer.spec.tsx(5 hunks)apps/deploy-web/src/components/alerts/ContactPointsListContainer/ContactPointsListContainer.tsx(1 hunks)apps/deploy-web/tests/unit/container-testing-child-capturer.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- apps/api/env/.env
🧰 Additional context used
🧬 Code Graph Analysis (2)
apps/deploy-web/src/components/alerts/ContactPointsListContainer/ContactPointsListContainer.tsx (1)
apps/deploy-web/src/components/alerts/ContactPointCreateContainer/ContactPointCreateContainer.tsx (1)
ChildrenProps(17-20)
apps/deploy-web/src/components/alerts/ContactPointCreateContainer/ContactPointCreateContainer.tsx (1)
apps/deploy-web/src/components/alerts/ContactPointsListContainer/ContactPointsListContainer.tsx (1)
ChildrenProps(15-23)
🔇 Additional comments (16)
apps/deploy-web/tests/unit/container-testing-child-capturer.tsx (1)
25-27: Well-designed factory function.The factory function provides a clean API for creating instances while maintaining type safety through generics.
apps/deploy-web/src/components/alerts/ContactPointsListContainer/ContactPointsListContainer.tsx (1)
15-15: Exporting internal types for testing is acceptable.Exporting the
ChildrenPropstype enables type-safe testing with the new testing utility. While this slightly breaks encapsulation, it's a reasonable trade-off for improved test quality and type safety.apps/deploy-web/src/components/alerts/ContactPointCreateContainer/ContactPointCreateContainer.tsx (1)
17-17: Consistent pattern for enabling testability.Exporting the
ChildrenPropstype follows the same pattern asContactPointsListContainer, maintaining consistency across container components. This enables type-safe interaction with the child render prop in tests.apps/deploy-web/src/components/alerts/ContactPointCreateContainer/ContactPointFormContainer.spec.tsx (7)
10-10: Good use of exported type for type safety.Importing the
ChildrenPropstype enables type-safe interaction with the container's child render prop, improving test reliability and developer experience.
15-16: Clean import organization.The imports are well-organized, removing unused
fireEventand adding the necessary testing utility. The testing utility import follows a logical path structure.
20-22: Improved testing approach with direct method invocation.The refactor from DOM event simulation to direct method calls makes the test more focused on the container's logic rather than UI interactions. This is generally better for unit testing container components.
75-75: Appropriate async setup function.Making the setup function async is necessary to await the child capturer, ensuring the child render prop is available before tests proceed.
101-101: Good use of type parameter for type safety.The generic type parameter
<ChildrenProps>ensures type safety when capturing and using the child render prop methods.
107-107: Clean integration of testing utility.Passing
childCapturer.renderChildas the children function cleanly integrates the testing utility with the container component.
113-113: Proper async handling in setup.The setup function correctly awaits the child capturer and returns the captured child along with other test dependencies, enabling clean test execution.
apps/deploy-web/src/components/alerts/ContactPointsListContainer/ContactPointsListContainer.spec.tsx (6)
10-10: LGTM! Import changes align with the new testing approach.The import modifications correctly reflect the shift from DOM-driven to prop-driven testing:
- Added
ContactPointsListViewPropstype for proper typing- Removed
fireEventas DOM events are no longer simulated- Added the child capturer utility for the new testing pattern
Also applies to: 14-14, 16-16
20-21: LGTM! Direct prop assertion is more reliable.The change from DOM text assertions to direct data prop comparison is an improvement:
- More deterministic than DOM content checking
- Tests the actual data flow rather than rendered output
- Eliminates potential flakiness from DOM rendering timing
25-26: LGTM! Direct method invocation improves test reliability.The shift from
fireEvent.click()to directchild.onRemove()invocation provides several benefits:
- Tests the actual callback behavior without DOM interaction complexity
- More focused unit testing of container logic
- Eliminates potential timing issues with DOM events
- Maintains the same test coverage for success and error scenarios
Also applies to: 38-40
52-53: LGTM! Pagination testing is well-implemented.The direct invocation of
child.onPaginationChange()with calculated pagination parameters is correct and maintains test coverage for pagination logic.
70-70: LGTM! Async setup function is necessary for the new testing pattern.Converting the setup function to async is required to properly await the child capturer initialization and data loading.
95-95: LGTM! Child capturer implementation follows the expected pattern.The implementation correctly:
- Creates a typed capturer for
ContactPointsListViewProps- Passes the capturer's render function to the container
- Awaits child initialization with a proper condition (
({ data }) => !!data.length)- Returns the captured child for test usage
This pattern provides a clean way to test container logic without DOM complexity.
Also applies to: 101-101, 107-109
...loy-web/src/components/alerts/ContactPointsListContainer/ContactPointsListContainer.spec.tsx
Show resolved
Hide resolved
2 participants
This is a draft attempting to simplify container testing via providing a simple object as a child which can be used to call container methods.
Summary by CodeRabbit