-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: enzyme to RTL (some files) #4559
Conversation
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 pull request was sent to the PullRequest network.
@yuki0410-dev you can click here to see the review status or cancel the code review job.
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.
PullRequest Breakdown
Reviewable lines of change
+ 349
- 237
99% JavaScript (tests)
1% JavaScript
Type of change
Fix - These changes are likely to be fixing a bug or issue.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4559 +/- ##
=======================================
Coverage 95.65% 95.65%
=======================================
Files 29 29
Lines 2579 2579
Branches 1069 1069
=======================================
Hits 2467 2467
Misses 112 112 ☔ View full report in Codecov by Sentry. |
@@ -189,7 +189,10 @@ export default class DatePicker extends React.Component { | |||
injectTimes: PropTypes.array, | |||
inline: PropTypes.bool, | |||
isClearable: PropTypes.bool, | |||
toggleCalendarOnIconClick: PropTypes.func, | |||
toggleCalendarOnIconClick: PropTypes.oneOfType([ |
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.
Fixed due to deviation from the default value, type bool.
This is great, and appreciate the effort! Ideally we would remove all enzyme usage from this repo. |
When we heard that the new tests were going to use RTL instead of enzyme, we decided that we needed to rewrite the existing tests with RTL.
Since I do not have the time to change everything at once, I created this PR to try to change things little by little.
(Sorry if there is an existing issue, PR.)
ref #4300