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.

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

Add toBeValidDate matcher #133

Merged

Conversation

tejasbubane
Copy link
Contributor

Issue: #117

What

Add new matcher toBeValidDate.

Why

As suggested in the issue.

Notes

Housekeeping

  • Unit tests
  • Documentation is up to date
  • No additional lint warnings
  • Add yourself to contributors list (yarn contributor)
  • Typescript definitions are added/updated where relevant

@tejasbubane tejasbubane force-pushed the add-tobe-valid-date-matcher branch 8 times, most recently from 24e9099 to 374d019 Compare June 2, 2018 17:53
@codecov-io
Copy link

codecov-io commented Jun 2, 2018

Codecov Report

Merging #133 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #133   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          90     92    +2     
  Lines         425    434    +9     
  Branches       70     72    +2     
=====================================
+ Hits          425    434    +9
Impacted Files Coverage Δ
src/matchers/toBeValidDate/index.js 100% <100%> (ø)
src/matchers/toBeValidDate/predicate.js 100% <100%> (ø)
src/utils/index.js 100% <100%> (ø) ⬆️
src/matchers/toBeDate/predicate.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5ffe04c...01a6dca. Read the comment docs.

@@ -0,0 +1,7 @@
let is = type => value => Object.prototype.toString.call(value) === `[object ${type}]`;
Copy link
Member

Choose a reason for hiding this comment

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

This might be a useful function to pull out into utils and then import here


let isValidDate = value => hasDateType(value) && !isNaN(value) && !isNaN(value.getTime());

export default expected => isValidDate(expected);
Copy link
Member

Choose a reason for hiding this comment

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

This can just be:

export default isValidDate;

We don't need both the left and right hand sides of the invocation :)

@@ -0,0 +1,7 @@
let is = type => value => Object.prototype.toString.call(value) === `[object ${type}]`;

let hasDateType = is('Date');
Copy link
Member

Choose a reason for hiding this comment

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

Would it be possible to use const for consistency with other matchers?

@mattphillips
Copy link
Member

Love this @tejasbubane I've added a couple of minor nitpicks but looks awesome! Would you be able to merge master in too to fix the conflicts?

@tejasbubane
Copy link
Contributor Author

tejasbubane commented Jun 16, 2018

@mattphillips I have updated both toBeDate and toBeValidDate matchers as per your comments.

@mattphillips mattphillips merged commit 01a6dca into jest-community:master Jul 30, 2018
@mattphillips
Copy link
Member

Thanks @tejasbubane

@tejasbubane tejasbubane deleted the add-tobe-valid-date-matcher branch July 30, 2018 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants