olivernn / lunr.js

A bit like Solr, but much smaller and not as bright
http://lunrjs.com
MIT License
8.96k stars 548 forks source link

Test error #75

Closed cambridgemike closed 9 years ago

cambridgemike commented 10 years ago

I just forked the repository (everything is up to date), but I'm getting a test failure. Is this a known test failure or do I have something misconfigured? Was there supposed to be a build step before running the tests?

~/code/lunr.js (master) $ make test Test failed: lunr.tokenizer: calling to string on passed val Failed assertion: expected: tue,jan,01,2013, but was: mon,dec,31,2012 at http://localhost:32423/test/env/qunit.js:472 at http://localhost:32423/test/tokenizer_test.js:50 at http://localhost:32423/test/env/qunit.js:136 at http://localhost:32423/test/env/qunit.js:279 at process (http://localhost:32423/test/env/qunit.js:1277) at http://localhost:32423/test/env/qunit.js:383 Took 58ms to run 297 tests. 296 passed, 1 failed. make: *\ [test] Error 1

olivernn commented 10 years ago

It's not a known test failure and the only set up step you missed was being in a timezone far from UTC :wink:

lunr.tokenizer will convert any object (except arrays) you pass to it into a string that can later be indexed. It does this by calling toString on the passed object.

In JavaScript, calling toString on a date will convert it into your local timezone before generating the string representation. When you run the tests, I presume at some negative offset from UTC, it will convert the UTC date into your local date time, meaning that it is actually the previous day (and year in this case).

So, what should the correct behaviour be? Is the test or implementation wrong? I think the test is wrong here, if I give lunr a date to index I don't think it should be changing the timezone of the date before indexing. Instead of trying to create a date from UTC the test should just create a date in the local timezone, I think.

I'll fix the test so that this doesn't happen, thanks for taking the time to point it out, timezones are tricky!

smikes commented 9 years ago

Is this believed fixed or still open? Just ran into it today, timezone is America/Edmonton, reproduce it like this:

$ TZ=/usr/share/zoneinfo/America/Edmonton  npm test

> lunr@0.5.7 test /Users/smikes/src/github/lunr.js
> make test

Starting test server at http://localhost:32423
Test failed: lunr.tokenizer: calling to string on passed val
    Failed assertion: expected: tue,jan,01,2013, but was: mon,dec,31,2012
    at http://localhost:32423/test/env/qunit.js:472
    at http://localhost:32423/test/tokenizer_test.js:50
    at http://localhost:32423/test/env/qunit.js:136
    at http://localhost:32423/test/env/qunit.js:279
    at process (http://localhost:32423/test/env/qunit.js:1277)
    at http://localhost:32423/test/env/qunit.js:383
Took 54ms to run 301 tests. 300 passed, 1 failed.

All tests pass if I use a TZ east of prime meridian, eg TZ=/usr/share/zoneinfo/Europe/Paris

olivernn commented 9 years ago

There has been another fix for this issue pushed in the latest release, 0.5.8.

Feel free to re-open if you continue to see similar issues.