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 value test failures with Node.js version 8.9.1 #475

Closed
wooldridge opened this issue Dec 4, 2017 · 5 comments
Closed

Date value test failures with Node.js version 8.9.1 #475

wooldridge opened this issue Dec 4, 2017 · 5 comments
Assignees

Comments

@wooldridge
Copy link
Contributor

wooldridge commented Dec 4, 2017

Upgrading to Node.js version 8.9.1 and running test-basic on the develop branch yields the following two failures (these failures do not occur with Node.js version 6.9.0):

805 passing (1m)
  2 failing

  1) server-side call
       to eval JavaScript
         should generate a date value:

      AssertionError: expected 2010-10-08 10:17:15.125 -0700 to equal 2010-10-08 03:17:15.125 -0700
      + expected - actual

      -[Date: 2010-10-08T17:17:15.125Z]
      +[Date: 2010-10-08T10:17:15.125Z]
      
      at Assertion.fail (node_modules/should/cjs/should.js:275:17)
      at Assertion.value (node_modules/should/cjs/should.js:356:19)
      at test-basic/server-exec.js:109:32
      at tryCatcher (node_modules/bluebird/js/release/util.js:16:23)
      at Promise._settlePromiseFromHandler (node_modules/bluebird/js/release/promise.js:512:31)
      at Promise._settlePromise (node_modules/bluebird/js/release/promise.js:569:18)
      at Promise._settlePromise0 (node_modules/bluebird/js/release/promise.js:614:10)
      at Promise._settlePromises (node_modules/bluebird/js/release/promise.js:693:18)
      at Async._drainQueue (node_modules/bluebird/js/release/async.js:133:16)
      at Async._drainQueues (node_modules/bluebird/js/release/async.js:143:10)
      at Immediate.Async.drainQueues (node_modules/bluebird/js/release/async.js:17:14)

  2) server-side call
       to invoke JavaScript
         should return a date value:

      AssertionError: expected 2010-10-08 10:17:15.125 -0700 to equal 2010-10-08 03:17:15.125 -0700
      + expected - actual

      -[Date: 2010-10-08T17:17:15.125Z]
      +[Date: 2010-10-08T10:17:15.125Z]
      
      at Assertion.fail (node_modules/should/cjs/should.js:275:17)
      at Assertion.value (node_modules/should/cjs/should.js:356:19)
      at test-basic/server-exec.js:343:32
      at tryCatcher (node_modules/bluebird/js/release/util.js:16:23)
      at Promise._settlePromiseFromHandler (node_modules/bluebird/js/release/promise.js:512:31)
      at Promise._settlePromise (node_modules/bluebird/js/release/promise.js:569:18)
      at Promise._settlePromise0 (node_modules/bluebird/js/release/promise.js:614:10)
      at Promise._settlePromises (node_modules/bluebird/js/release/promise.js:693:18)
      at Async._drainQueue (node_modules/bluebird/js/release/async.js:133:16)
      at Async._drainQueues (node_modules/bluebird/js/release/async.js:143:10)
      at Immediate.Async.drainQueues (node_modules/bluebird/js/release/async.js:17:14)
@ehennum
Copy link
Contributor

ehennum commented Dec 4, 2017

Maybe a time zone issue of some kind -- given that both are 7 hours off.

@wooldridge
Copy link
Contributor Author

These failures are due to a change in how dates are handled in Node.js in version 8 (on account of the underlying ES2015 spec). See here for details:

nodejs/node#13210

When the following is eval'ed via the Node.js Client API:

new Date("2010-10-08T10:17:15.125Z)

the MarkLogic server returns a date without a time zone:

2010-10-08T10:17:15.125

The ES2015 spec says:

When the time zone offset is absent, date-only forms are interpreted as a UTC time and date-time forms are interpreted as a local time.

Thus the following line in the API code:

https://github.com/marklogic/node-client-api/blob/master/lib/server-exec.js#L85

yields the following value, which is shifted based on the local time zone:

2010-10-08T17:17:15.125Z

This new behavior breaks the two tests, which compare that value with the following that uses a value (with a timezone):

new Date('2010-10-08T10:17:15.125Z')

A fix is to eval and test against the following, which avoids the time zone-related mismatch:

2010-10-08T10:17:15.125

wooldridge added a commit to wooldridge/node-client-api that referenced this issue Dec 5, 2017
Addresses change in date handling in Node.js version 8.
Fixes marklogic#475
wooldridge added a commit that referenced this issue Dec 5, 2017
Addresses change in date handling in Node.js version 8.
Fixes #475
@ehennum
Copy link
Contributor

ehennum commented Dec 5, 2017

Good sleuthing.

@wooldridge wooldridge added fix and removed new labels Dec 5, 2017
@wooldridge wooldridge self-assigned this Dec 5, 2017
wooldridge added a commit to wooldridge/node-client-api that referenced this issue Dec 5, 2017
Addresses change in date handling in Node.js version 8.
Fixes marklogic#475
wooldridge added a commit that referenced this issue Dec 5, 2017
Addresses change in date handling in Node.js version 8.
Fixes #475
@wooldridge
Copy link
Contributor Author

@ayuwono, FYI, in looking ahead to supporting Node.js versions 8 and beyond, you may have to adjust tests that eval dates on the server (and possibly other date tests).

@wooldridge
Copy link
Contributor Author

Merged after passing test-basic locally and on Jenkins.

@wooldridge wooldridge added ship and removed fix labels Dec 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants