Added tests for utils functions isSelectable and mergeModifiers
#9
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* ... This change addresses the need by: - add unit tests for `isSelectable` predicate utility - dates within range (inclusive of range dates) -> true - dates outside range dates -> false - add unit tests for `mergeModifiers` utility - documenting behaviour of merging distinct lists - documenting behaviour of lists containing same keys
hernansartorio
requested changes
Feb 4, 2020
| @@ -0,0 +1,77 @@ | |||
| // import React from 'react' | |||
| // import { format } from 'date-fns' | |||
There was a problem hiding this comment.
You can remove these comments.
| expect(isSelectable(minimumDate, { minimumDate, maximumDate })).toBe(true) | ||
| expect(isSelectable(maximumDate, { minimumDate, maximumDate })).toBe(true) | ||
| }) | ||
| it('should return `false` for a date that is before `minimumDate` or after `maximumDate`', () => { |
There was a problem hiding this comment.
Purely stylistic (I should add an eslint rule for this) but could you leave a blank line between the end of a block and the start of a new one? In the same way that you do within the mergeModifiers tests.
| expect(mergeModifiers(mockBaseModifiers)).toEqual(mockBaseModifiers) | ||
| }) | ||
|
|
||
| it('should (in effect) merge a provided modifier into the `modifiers` object if the key is not already present', () => { |
There was a problem hiding this comment.
What does "in effect" mean?
| }) | ||
| }) | ||
|
|
||
| it('should check both `baseModifier` and `newModifier` of the same name', () => { |
There was a problem hiding this comment.
I think a better way to test this one, without worrying about what values are passed, might be using toHaveBeenCalled, something like:
const modifierA = jest.fn()
const modifierB = jest.fn()
const baseModifiers = { test: modifierA }
const newModifiers = { test: modifierB }
const mergedModfiers = mergeModifiers(baseModifiers, newModifiers)
mergedModfiers.test()
expect(modifierA).toHaveBeenCalled()
expect(modifierB).toHaveBeenCalled()
| expect(mergedModfiers.test(NOT_A_NUMBER_OVER_1_OR_A_STRING)).toBe(false) | ||
| }) | ||
|
|
||
| it('should prefer a `baseModifier` with the same name as a `newModifier`', () => { |
There was a problem hiding this comment.
Similarly, this one could be like this:
const modifierA = jest.fn(() => true)
const modifierB = jest.fn()
const baseModifiers = { test: modifierA }
const newModifiers = { test: modifierB }
const mergedModfiers = mergeModifiers(baseModifiers, newModifiers)
mergedModfiers.test()
expect(modifierA).toHaveBeenCalled()
expect(modifierB).not.toHaveBeenCalled()
| expect(mergedModfiers.today('which one?')).toBe('baseModifier') | ||
| }) | ||
|
|
||
| it('should assign the `newModifier` if no `baseModifier` of the same name is provided', () => { |
There was a problem hiding this comment.
Isn't what's tested here already covered by the should (in effect) merge a provided modifier into the 'modifiers' object if the key is not already present test?
vibin-230
pushed a commit
to vibin-230/react-nice-dates
that referenced
this pull request
Feb 2, 2021
fixes time zone issues
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change addresses the need by:
isSelectablepredicate utilitymergeModifiersutility