test(Tasks): Add Recur, Priority, and Reports toggle tests #436
Conversation
|
Thank you for opening this PR! Before a maintainer takes a look, it would be really helpful if you could walk through your changes using GitHub's review tools. Please take a moment to:
More information on how to conduct a self review: This helps make the review process smoother and gives us a clearer understanding of your thought process. Once you've added your self-review, we'll continue from our side. Thank you! |
- Add feature-specific tests for editing flows - Move data-testid from SelectTrigger to Select for testability - Update Select mock to support dynamic test-ids - Add ResizeObserver mock
ShivaGupta-14
force-pushed
the
test/300-tasks-recur-priority-reports
branch
from
63ea37c to
e78b544
Compare
There was a problem hiding this comment.
Moreover I'd suggest creating separate PRs for Recur, Priority, Reports toggle. And so on. Most tests new or old end up being flaky so we need a better way to sort these out.
Better yet just create a new file for those tests if they are new, for ex:
Tasks.editing.test.tsx
Tasks.priority.test.tsx and similar.
Else if the suite already exists, and need to update anything, do so in the same file. Will break this down later
|
|
||
| test('shows error toast when priority save fails', async () => { | ||
| const { toast } = require('react-toastify'); | ||
| const hooks = require('../hooks'); |
There was a problem hiding this comment.
imo just move this to the top, its being reused a lot
| test('saving "NONE" priority sends empty string to backend', async () => { | ||
| render(<Tasks {...mockProps} />); | ||
|
|
||
| await screen.findByText('Task 12'); |
There was a problem hiding this comment.
We have too many tests, some are vague as well, and it will be even more hard to distinguish later.
Do move blocks like these to separate functions
await screen.findByText('Task 12');
fireEvent.click(screen.getByText('Task 12'));
expect(await screen.findByText('Priority:')).toBeInTheDocument();
There was a problem hiding this comment.
got it! is this good structure, should i proceed with this
frontend/src/components/HomeComponents/Tasks/
├── __tests__/
├── setup.ts -> mocks
├── helpers.ts -> shared utilities
├── Tasks.test.tsx
├── Tasks.keyboard.test.tsx
├── Tasks.priority.test.tsx
├── Tasks.recur.test.tsx
├── Tasks.reports.test.tsx
├── Tasks.annotations.test.tsx
I'll extract repeated code (like opening task dialog) into helpers.ts.
- Also, could you point out which tests you found vague? I'd like to improve those
| Select: ({ children, onValueChange, value, ...props }: any) => { | ||
| return ( | ||
| <select | ||
| data-testid="project-select" |
There was a problem hiding this comment.
these data-testid related changes are not really desired.
should be reverted
2 participants
Description
Add feature-specific tests for editing flows
Move data-testid from SelectTrigger to Select for testability
Update Select mock to support dynamic test-ids
Add ResizeObserver mock
Tracking: Tracking Issue: Tasks Component Test Improvements #433
Checklist
npx prettier --write .(for formatting)gofmt -w .(for Go backend)npm test(for JS/TS testing)Additional Notes
Before:
After: