Fix currently failing tests #4564
Conversation
benmccann
changed the title
test/specs/scale.time.tests.js
Outdated
| @@ -297,8 +297,8 @@ describe('Time scale tests', function() { | |||
| var xScale = chart.scales.xScale0; | |||
|
|
|||
| // Counts down because the lines are drawn top to bottom | |||
| expect(xScale.ticks[0].value).toBe('Jan 2'); | |||
| expect(xScale.ticks[1].value).toBe('May 8'); | |||
| expect(xScale.ticks[0].value).toEqualOneOf(['Jan 1', 'Jan 2']); | |||
There was a problem hiding this comment.
Would be better to not have approximative expectations. The failure might be because we use timestamps to create the moment object (line 284-285), we could try to use string instead (as other tests don't fail):
parser: function(label) {
return label === 'foo' ?
moment('2000/01/02', 'YYYY/MM/DD') :
moment('2016/05/08', 'YYYY/MM/DD');
}There was a problem hiding this comment.
I'm not sure what the root cause is here, but my guess is it's timezone related? Changing the parser function didn't seem to fix it.
Reverting to the previous state seems like an improvement to me even if there's ultimately a better fix. I'd rather spend my time fixing issues that affect more people than invest more time in this
There was a problem hiding this comment.
I have been able to reproduce this issue locally by modifying my local time zone. Changing the parser (as above) fixes the issue for me.
There was a problem hiding this comment.
Strange. Now it's passing for me the way you suggested and failing with the timestamps version. I swear I tried that earlier. Anyway, glad it's passing now. Thanks for the help!
.travis.yml
Outdated
| - google-chrome | ||
| packages: | ||
| - google-chrome-stable | ||
| chrome: beta # stable version (60.0.3112.78) is bugged |
There was a problem hiding this comment.
What's the bug exactly? are we sure it's not our code that doesn't work against this version?
There was a problem hiding this comment.
I'm not sure. I've updated the comment as I can't be positive if the bug is in our code or in Chrome. It is odd that Chart.js works with Chrome 59 and 61, but not 60. That was one reason I thought the issue was in Chrome. This only fixes the issue on Travis. Tests will still fail locally. But it will at least let us use Travis to evaluate incoming PRs.
benmccann
force-pushed
the
fixTimeTests
branch
3 times, most recently
from
49175c2 to
ad60343
Compare
|
Merged with 2cc242d |
@simonbrunel looks like two of the tests got broken here: 3aa7a20#diff-32e22477b9f6458d20701ea5c8720ea4L292
The other 4 are failing because Chrome 60.0.3112 was released today
I'm not sure that these are the best fixes and haven't investigated underlying root causes, but still would appreciate merging them to get Travis back to full health in the meantime