Skip to content

Fix failing unit tests related to time widget#1119

Merged
edi9999 merged 1 commit intorjsf-team:masterfrom
LucianBuzzo:fix-failing-tests
Jan 7, 2019
Merged

Fix failing unit tests related to time widget#1119
edi9999 merged 1 commit intorjsf-team:masterfrom
LucianBuzzo:fix-failing-tests

Conversation

@LucianBuzzo
Copy link
Copy Markdown
Collaborator

@LucianBuzzo LucianBuzzo commented Jan 3, 2019

Reasons for making this change

It appears that due to the recent change from 2018 to 2019, there is one additional year in the select list, which caused the unit tests to fail.

Checklist

  • I'm updating documentation
    • I've checked the rendering of the Markdown text I've added
    • If I'm adding a new section, I've updated the Table of Content
  • I'm adding or updating code
    • I've added and/or updated tests
    • I've updated docs if needed
    • I've run npm run cs-format on my branch to conform my code to prettier coding style
  • I'm adding a new feature
    • I've updated the playground with an example use of the feature

@edi9999
Copy link
Copy Markdown
Collaborator

edi9999 commented Jan 7, 2019

For some reason this failed on travis

@LucianBuzzo
Copy link
Copy Markdown
Collaborator Author

@edi9999 Strange, I'm going to try and debug it

@edi9999
Copy link
Copy Markdown
Collaborator

edi9999 commented Jan 7, 2019

One idea of mine is that for some reason new Date().getFullYear() returns 2018.

Can you add a console.log(new Date().getFullYear()) and console.log(new Date()) for this ?

@LucianBuzzo
Copy link
Copy Markdown
Collaborator Author

@edi9999 The test passes when using .only, and the available options appear to be the same in both cases:

@LucianBuzzo
Copy link
Copy Markdown
Collaborator Author

@edi9999 I was being stupid, it was actually another test that was failing -> https://github.com/mozilla-services/react-jsonschema-form/blob/master/test/StringField_test.js#L1007 I'll update it with the same fix

It appears that due to the recent change from 2018 to 2019, there is one
additional year in the select list, which caused the unit tests to fail.

Signed-off-by: Lucian <lucian.buzzo@gmail.com>
@edi9999 edi9999 merged commit 4b9c475 into rjsf-team:master Jan 7, 2019
@edi9999
Copy link
Copy Markdown
Collaborator

edi9999 commented Jan 7, 2019

Thanks @LucianBuzzo !


expect(lengths).eql([
121 + 1, // from 1900 to 2020 + undefined
// from 1900 to current year + 2 (inclusive) + 1 undefined option
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

A nitpick - but would it make sense to extract a function that provides this, to avoid duplication between tests ?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

That is imo not really necessary, in tests duplication is not so bad.

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