-
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
Attempt to fix spec that breaks on last day of the month #4541
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.
Check the status or cancel PullRequest code review here.
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
+ 1
- 1
100% JavaScript (tests)
Type of change
Fix - These changes are likely to be fixing a bug or issue.
1 Message | |
---|---|
📚 | It looks like the description for this pull request is either blank or very short. Adding a high-level summary will help our reviewers provide better feedback. Feel free to include questions for PullRequest reviewers and make specific feedback requests. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4541 +/- ##
=======================================
Coverage 95.45% 95.45%
=======================================
Files 29 29
Lines 2533 2533
Branches 1039 1039
=======================================
Hits 2418 2418
Misses 115 115 ☔ View full report in Codecov by Sentry. |
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.
You may not choose to take action this commit, but I suggested you might consider expanding your use of fixed reference dates in your tests.
Reviewed with ❤️ by PullRequest
@@ -2102,7 +2102,7 @@ describe("DatePicker", () => { | |||
}); | |||
|
|||
it("should hide the calendar even when the startDate and the endDate is same in the range", async () => { | |||
let startDate = new Date(); | |||
let startDate = new Date("2021-01-21 00:00:00"); |
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.
Like you, I have found that using specific dates in my test cases to be superior to using dates that follow the date of execution of the test.
It may be possible that most of your tests would benefit from having a fixed reference date to prove behavior, especially around edge cases.
◽ Giving Information
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 agreed, I will revisit the test to see if we can apply this to more cases. Using a random date or today's date causes a lot of uncertainty in the test cases.
No description provided.