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
fix: correct SmallDateTime upper bound range #1621
base: master
Are you sure you want to change the base?
Conversation
5ae126f
to
3a58a61
Compare
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.
Thanks for catching this and puttying in the effort for raising this PR. I just had a comment for remove an unnecessary test case, and it should be good to go. 🙇
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.
Sorry, I may put my pervious comment at a confusing line. I added new comment, and hope it will clear things up.
|
||
assert.throws(() => { | ||
TYPES.SmallDateTime.validate(new Date('June 1, 2079')); | ||
}, TypeError, 'Out of range.'); |
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.
Sorry, Maybe I was being confusing on this. I mean that the test case that test against May 7, 2079 can be removed since it this date is a valid smalldatetime value. Your original modification that change June 1, 2079 to June 7, 2079 is still a valid modification we should keep that.
Basically this test will looks like this:
describe('.validate', function() {
it.only('returns a TypeError for dates that are out of range', function() {
assert.throws(() => {
TYPES.SmallDateTime.validate(new Date('Dec 31, 1889'));
}, TypeError, 'Out of range.');
assert.throws(() => {
TYPES.SmallDateTime.validate(new Date('Jan 1, 2080'));
}, TypeError, 'Out of range.');
assert.throws(() => {
TYPES.SmallDateTime.validate(new Date('June 7, 2079'));
}, TypeError, 'Out of range.');
});
});
Before submitting a PR :
master
branch of the repository.npm install
in the root folder.npm run-script test-all
). During development, to run individual test usenode_modules/nodeunit test/<test_file.js> -t <test_name>
.npm run build
).npm run lint
).node_modules/.bin/commitlint --from origin/master --to HEAD
). Refer commit conventions and commit rules.Thank you for Contributing!