test(workflow): improve dataset item tests with edit and remove functionality#33937
test(workflow): improve dataset item tests with edit and remove functionality#33937
Conversation
…ionality - Enhanced integration tests for the DatasetItem component to verify editing and removing dataset items. - Updated test cases to utilize userEvent and waitFor for better async handling. - Added aria-labels and data-testid attributes to action buttons for improved accessibility and testing.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the integration tests for the DatasetItem component by adding specific tests for editing and removing dataset items. It also improves the test's async handling and adds accessibility attributes. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Pull request overview
Improves the workflow “knowledge-retrieval” frontend test suite around dataset item interactions, while also making the DatasetItem action buttons more accessible and easier to target in tests.
Changes:
- Added
aria-labelanddata-testidattributes to the DatasetItem edit/remove action buttons. - Refined integration tests to separately verify “edit” and “remove” flows using
userEvent,within, andwaitFor. - Introduced a small test helper (
getDatasetItem) to reduce repeated DOM traversal logic.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
web/app/components/workflow/nodes/knowledge-retrieval/components/dataset-item.tsx |
Adds accessible names and stable test selectors to icon-only action buttons. |
web/app/components/workflow/nodes/knowledge-retrieval/__tests__/integration.spec.tsx |
Updates dataset item integration tests to target edit/remove actions more explicitly and handle async updates more reliably. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Code Review
This pull request improves the integration tests for the DatasetItem component by splitting a large test into two more focused ones and using more robust queries. It also enhances accessibility by adding aria-labels to action buttons. My review focuses on further improving the test robustness and cleaning up redundant test attributes. I've suggested refining the helper function for querying the DOM to be less brittle and removing unused data-testid attributes since the tests now correctly use accessibility labels for selection.
web/app/components/workflow/nodes/knowledge-retrieval/__tests__/integration.spec.tsx
Show resolved
Hide resolved
web/app/components/workflow/nodes/knowledge-retrieval/components/dataset-item.tsx
Show resolved
Hide resolved
web/app/components/workflow/nodes/knowledge-retrieval/components/dataset-item.tsx
Show resolved
Hide resolved
…nd removal - Updated the MockSettingsModal to use useEffect and useRef for improved state management during tests. - Simplified test cases by removing unnecessary userEvent setup and mouseOver events. - Enhanced clarity and maintainability of the integration tests for dataset item operations.
…omponent - Updated the resize timer and chart ready timer to use clearer cleanup functions. - Refactored the debounced resize and chart ready handling to enhance performance and prevent memory leaks. - Added console error and warning spies in tests for better error handling during component testing.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
web/app/components/workflow/nodes/knowledge-retrieval/components/dataset-item.tsx
Show resolved
Hide resolved
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #33937 +/- ##
=======================================
Coverage 77.90% 77.90%
=======================================
Files 4578 4578
Lines 181219 181238 +19
Branches 35347 35347
=======================================
+ Hits 141179 141197 +18
- Misses 36794 36795 +1
Partials 3246 3246
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Important
Fixes #<issue number>.Summary
test(workflow): improve dataset item tests with edit and remove functionality
Checklist
make lintandmake type-check(backend) andcd web && npx lint-staged(frontend) to appease the lint gods