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

Date toString tests #930

Merged
merged 3 commits into from
Jun 22, 2017
Merged

Date toString tests #930

merged 3 commits into from
Jun 22, 2017

Conversation

littledan
Copy link
Member

These tests check the semantics of Date.prototype.toString, Date.prototype.toTimeString,
Date.prototype.toDateString and Date.prototype.toUTCString. There are probably ways
to make the more specific, but this is a start. The semantics are currently implementation-defined,
but a spec pull request at tc39/ecma262#848 gives them this behavior.

Copy link
Member

@leobalter leobalter left a comment

Choose a reason for hiding this comment

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

Not a final review, but I just found this bug.

assert.notSameValue(null, match);

// Years are padded to the left with zeroes
match = stringRegExp.exec(new Date('0020-01-01T00:00:00Z').toDateString());
Copy link
Member

Choose a reason for hiding this comment

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

s/stringRegExp/dateRegExp

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@littledan
Copy link
Member Author

The spec change landed, but changes still needed on the test.

@littledan
Copy link
Member Author

With that typo fixed, the tests pass on a hacked version of V8 that implements the new specification, so I don't think there are any more obvious typos.

@rwaldron rwaldron dismissed leobalter’s stale review June 22, 2017 20:22

Review was addressed

@rwaldron rwaldron merged commit e3fa665 into tc39:master Jun 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants