- Notifications
You must be signed in to change notification settings - Fork1k
feat: add tests for dynamic parameters#18679
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
82e1666
to3d3ccc3
Compare<title>{pageTitle(title)}</title> | ||
</Helmet> | ||
{!latestResponse|| | ||
{(!latestResponse&&!wsError)|| |
Kira-PilotJul 17, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is an actual fix, right? Also, should we not guard against rendering if either one of these conditions is missing (regardless of whether they're missing together)?
Lastly, this boolean logic is getting difficult to reason about; I wonder if it's time to pull into a distinct boolean.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
yes, this just makes things a bit more specific to only wait for the latestResponse if there is not a websocket error.
site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.test.tsx OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.test.tsx OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.test.tsx OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
I think a 1000+ LOC testing PR is difficult to review thoroughly. Your websocket mock could be a PR by itself, as could your fixtures in the |
@CodeRabbit review |
✅ Actions performedReview triggered.
|
coderabbitaibot commentedJul 21, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
WalkthroughThe changes introduce a new Changes
Sequence Diagram(s)sequenceDiagram participant User participant TestSuite participant MockWebSocket participant CreateWorkspacePageExperimental User->>TestSuite: Run test TestSuite->>MockWebSocket: Create mock connection MockWebSocket->>CreateWorkspacePageExperimental: Simulate open event CreateWorkspacePageExperimental->>MockWebSocket: Listen for messages MockWebSocket->>CreateWorkspacePageExperimental: Send dynamic parameter data User->>CreateWorkspacePageExperimental: Interact with form fields CreateWorkspacePageExperimental->>MockWebSocket: Send parameter updates MockWebSocket->>CreateWorkspacePageExperimental: Simulate error/close events CreateWorkspacePageExperimental->>TestSuite: Render error or loader based on wsError/latestResponse User->>CreateWorkspacePageExperimental: Submit form CreateWorkspacePageExperimental->>TestSuite: Navigate or display errors Estimated code review effort🎯 4 (Complex) | ⏱️ ~90 minutes Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (4)site/**/*.tsx📄 CodeRabbit Inference Engine (.cursorrules)
Files:
site/src/**/*.test.{ts,tsx,js,jsx}📄 CodeRabbit Inference Engine (.cursorrules)
Files:
site/src/**/*.tsx📄 CodeRabbit Inference Engine (site/CLAUDE.md)
Files:
site/src/**/*.{ts,tsx}📄 CodeRabbit Inference Engine (site/CLAUDE.md)
Files:
🧠 Learnings (2)📓 Common learnings
site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.test.tsx (6)Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (8)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat withCodeRabbit:
SupportNeed help? Create a ticket on oursupport 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 ( |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.test.tsx OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.test.tsx OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.test.tsx OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.test.tsx OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.test.tsx OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.test.tsx OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
looking good!
some of the smaller rendering focused tests seem like they might be better off as storybook tests tho (and that might be a good opportunity to split this pr up just a little further as well)
other than that, mostly small comments. just gonna wait until michael's websocket pr is finalized and merged to give the stamp.
constshouldShowLoader= | ||
!templateQuery.data|| | ||
isLoadingFormData|| | ||
isLoadingExternalAuth|| | ||
autoCreateReady|| | ||
(!latestResponse&&!wsError); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
this expression still hurts my head, but thank you for moving it to a variable!
Uh oh!
There was an error while loading.Please reload this page.
site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.test.tsx OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.test.tsx OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.test.tsx OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.test.tsx OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.test.tsx OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.test.tsx OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.test.tsx OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.test.tsx OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
c2526a5
tod760feb
Compare@aslilac updated the tests to remove the queueMicrotask by moving the publish event outside of mockImplementation, below renderCreateWorkspacePageExperimental() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
looks great! ty!
0bfe0d6
intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Summary by CodeRabbit
New Features
Bug Fixes
Tests