Skip to content

Add property-based testing to temporal-types conversion #997

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

Merged

Conversation

bigmontz
Copy link
Contributor

@bigmontz bigmontz commented Sep 16, 2022

Add this type of testing to .toStandardDate() ('should be the reverse operation of fromStandardDate but losing time information') helps to cover corner cases and solve special cases such:

  • Negative date time not being serialised correctly in the iso standard. Years should always have 6 digits and the signal in front for working correctly with negative years and high numbers. This also avoids the year 2000 problem. See, https://en.wikipedia.org/wiki/ISO_8601
  • Date.fromStandardDate factory was not taking in consideration the seconds contribution in the timezone offset. This is not a quite common scenario, but there are dates with timezone offset of for example 50 minutes and 20 seconds.
  • Fix Date.toStandardDate for dates with offsets of seconds. Javascript Date constructor doesn't create dates from iso strings with seconds in the offset. For instance, new Date("2010-01-12T14:44:53+00:00:10"). So, the date should be re-created from the UTC timestamp.

Add this type of testing to `.toStandardDate()` ('should be the reverse operation of fromStandardDate but losing time information') helps to cover corner cases and solve special cases such:

* Negative date time not being serialized correctly in the iso standard. Years should always have 6 digits and the signal in front for working correctly with negative years and high numbers.
This also avoids the year 2000 problem. See, https://en.wikipedia.org/wiki/ISO_8601
* `Date.fromStandardDate` factory was not taking in consideration the `seconds` contribuition in the timezone offset.
This is not a quite common scenarion, but there are dates with timezone offset of for example `50 minutes` and `20 seconds`.
* Fix `Date.toStandardDate` for dates with offsets of seconds.
Javascript Date contructor doesn't create dates from iso strings with seconds in the offset.
For instance, `new Date("2010-01-12T14:44:53+00:00:10")`.
So, the date should be re-created from the utc timestamp.
@bigmontz bigmontz force-pushed the 5.0-add-property-based-testing-to-temporal-types branch from fff9c6c to 2e3a622 Compare September 16, 2022 15:33
Copy link
Member

@robsdedude robsdedude left a comment

Choose a reason for hiding this comment

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

🕙

Copy link
Member

@robsdedude robsdedude left a comment

Choose a reason for hiding this comment

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

🚀

@bigmontz bigmontz merged commit e269ab6 into neo4j:5.0 Sep 19, 2022
@bigmontz bigmontz deleted the 5.0-add-property-based-testing-to-temporal-types branch September 19, 2022 16:27
bigmontz added a commit to bigmontz/neo4j-javascript-driver that referenced this pull request Sep 20, 2022
Add this type of testing to `.toString()` ('should be serialize string which can be loaded by new Date') helps to cover corner cases and solve special cases such:

* Negative date time not being serialised correctly in the iso standard. Years should always have 6 digits and the signal in front for working correctly with negative years and high numbers. This also avoids the year 2000 problem. See, https://en.wikipedia.org/wiki/ISO_8601
* `Date.fromStandardDate` factory was not taking in consideration the `seconds` contribution in the timezone offset. This is not a quite common scenario, but there are dates with timezone offset of for example `50 minutes` and `20 seconds`.
* Fix `Date.toString` for dates with offsets of seconds. Javascript Date constructor doesn't create dates from iso strings with seconds in the offset. For instance, `new Date("2010-01-12T14:44:53+00:00:10")`. So, this tests should be skipped.
bigmontz added a commit that referenced this pull request Sep 21, 2022
Add this type of testing to `.toString()` ('should be serialize string which can be loaded by new Date') helps to cover corner cases and solve special cases such:

* Negative date time not being serialised correctly in the iso standard. Years should always have 6 digits and the signal in front for working correctly with negative years and high numbers. This also avoids the year 2000 problem. See, https://en.wikipedia.org/wiki/ISO_8601
* `Date.fromStandardDate` factory was not taking in consideration the `seconds` contribution in the timezone offset. This is not a quite common scenario, but there are dates with timezone offset of for example `50 minutes` and `20 seconds`.
* Fix `Date.toString` for dates with offsets of seconds. Javascript Date constructor doesn't create dates from iso strings with seconds in the offset. For instance, `new Date("2010-01-12T14:44:53+00:00:10")`. So, this tests should be skipped.

Co-authored-by: Robsdedude <[email protected]>
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.

2 participants