Skip to content
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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Increase test coverage for /modules/views #3211

Merged
merged 9 commits into from
Jan 5, 2024
Merged

Increase test coverage for /modules/views #3211

merged 9 commits into from
Jan 5, 2024

Conversation

gitstart-twenty
Copy link
Contributor

@gitstart-twenty gitstart-twenty commented Jan 3, 2024 •
edited

COVERAGE REPORT

Screenshot 2024-01-03 at 11 56 54

Fixes #3008

gitstart-twenty and others added 2 commits January 3, 2024 09:00
@gitstart-twenty @v1b3m @RubensRafael
Increase test coverage for /modules/views
31dbef7 
Co-authored-by: v1b3m <vibenjamin6@gmail.com>
Co-authored-by: RubensRafael <rubensrafael2@live.com>
@gitstart-twenty @v1b3m @RubensRafael
fix failing test
efa62bc 
Co-authored-by: v1b3m <vibenjamin6@gmail.com>
Co-authored-by: RubensRafael <rubensrafael2@live.com>
@gitstart-twenty
Copy link
Contributor Author

savedViewSortsFamilySelector has no coverage because it is not used anywhere

@gitstart-twenty gitstart-twenty marked this pull request as ready for review January 3, 2024 09:15
@gitstart-twenty gitstart-twenty added PR: awaiting review scope: front Issues that are affecting the frontend side only labels Jan 3, 2024
charlesBochet
charlesBochet reviewed Jan 3, 2024
View reviewed changes
Copy link
Member

@charlesBochet charlesBochet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, interesting work. I'm planning to refactor view bar and this will be useful
I have left comments

packages/twenty-front/src/modules/views/hooks/__tests__/useViewBar.test.tsx Outdated
fieldName: 'view',
},
},
] as unknown as ViewField[];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks suspicious, we try to avoid this in the codebase

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

packages/twenty-front/src/modules/views/hooks/__tests__/useViewBar.test.tsx
@@ -0,0 +1,831 @@
import { act } from 'react-dom/test-utils';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test file is too long and should be broken in smaller files
Intersting work though

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright

gitstart-twenty and others added 4 commits January 4, 2024 08:08
@gitstart-twenty
Merge commit 'f36fa9aa147f216537ad3a51bec2913b0988a957' of https://gi…
11e5f27 
…thub.com/twentyhq/twenty into TWNTY-3008
@gitstart-twenty @v1b3m @RubensRafael
Refactor into smaller tests
5b35b75 
Co-authored-by: v1b3m <vibenjamin6@gmail.com>
Co-authored-by: RubensRafael <rubensrafael2@live.com>
@gitstart-twenty @v1b3m @RubensRafael
Refactor according to review
394457f 
Co-authored-by: v1b3m <vibenjamin6@gmail.com>
Co-authored-by: RubensRafael <rubensrafael2@live.com>
@gitstart-twenty @v1b3m @RubensRafael
fix linter
2e23ba3 
Co-authored-by: v1b3m <vibenjamin6@gmail.com>
Co-authored-by: RubensRafael <rubensrafael2@live.com>
charlesBochet
charlesBochet approved these changes Jan 5, 2024
View reviewed changes
Copy link
Member

@charlesBochet charlesBochet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@charlesBochet charlesBochet merged commit 80c1c9a into main Jan 5, 2024
13 checks passed
@charlesBochet charlesBochet deleted the TWNTY-3008 branch January 5, 2024 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@charlesBochet charlesBochet charlesBochet approved these changes

Assignees
No one assigned
Labels
PR: awaiting review scope: front Issues that are affecting the frontend side only
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Increase test coverage for /modules/views
2 participants
@gitstart-twenty @charlesBochet