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

dc.js 3.0 test suit does not like Stockholm #1420

Closed
kum-deepak opened this issue Apr 19, 2018 · 8 comments
Closed

dc.js 3.0 test suit does not like Stockholm #1420

kum-deepak opened this issue Apr 19, 2018 · 8 comments

Comments

@kum-deepak
Copy link
Collaborator

It does not like Paris and London as well. 😄

One date related test case fails.

@gordonwoodhull
Copy link
Contributor

gordonwoodhull commented Apr 19, 2018

All of our tests should run in GMT UTC - maybe this one uses local time?

@gordonwoodhull
Copy link
Contributor

I think all of these should be utc instead of time:

$ grep d3\.time[A-Z] spec/*
spec/bar-chart-spec.js:            var expectedStartDate = d3.timeDay.offset(date, -10);
spec/bar-chart-spec.js:            var expectedEndDate = d3.timeDay.offset(date, 10);
spec/bar-chart-spec.js:            var expectedStartDate = d3.timeMonth.offset(date, -2);
spec/bar-chart-spec.js:            var expectedEndDate = d3.timeMonth.offset(date, 2);
spec/biggish-data-spec.js:            row.day = d3.timeDay(row.dd);
spec/biggish-data-spec.js:            row.hour = d3.timeHour(row.dd);
spec/biggish-data-spec.js:        extentDay[1] = d3.timeDay.offset(extentDay[1], 1);
spec/biggish-data-spec.js:            .xUnits(d3.timeHours);
spec/biggish-data-spec.js:            .xUnits(d3.timeHours)
spec/biggish-data-spec.js:            .round(d3.timeHour.round)

These come from more recent tests in 9e66f33 and 875288b that I wrote after the great and painful UTC conversion some years back.

@gordonwoodhull
Copy link
Contributor

But those aren't failing. (@kum-deepak, please quote the specific test when reporting a test failure.)

Setting my clock to Oslo I'm seeing this one fail instead:

Firefox 59.0.0 (Mac OS X 10.13.0) dc.bubbleChart with a date-based scale with 2 month padding should stretch the domain appropriately FAILED

It's from around the same time as the bar chart spec, also to do with padding units.

@gordonwoodhull
Copy link
Contributor

Interesting! It's a bug in the time padding itself.

We have

// convert 'day' to 'timeDay' and similar
dc.utils.toTimeFunc = function (t) {
    return 'time' + t.charAt(0).toUpperCase() + t.slice(1);
};

I can fix the test by changing time to utc here. I guess we need a way to configure that string constant.

Although this dynamic search for time functions is kind of fishy in itself... shouldn't the padding unit be the time interval object, rather than the name of it?

@kum-deepak
Copy link
Collaborator Author

Wow! I did not expect such quick response. I had created the issue as a marker at an airport.

This code is new, earlier it was directly calling that function by name. I converted it as the function names changed in D3v4.

I am not entirely sure what is correct behavior here. We can keep it configurable, but setting the correct default will be crucial as majority of the users will not change the default.

@gordonwoodhull
Copy link
Contributor

@kum-deepak, do you happen to be in Stockholm, or were you just messing with time zones?

This is fixed in 3.0 alpha 11.

@gordonwoodhull
Copy link
Contributor

The first time I discovered the problem of testing in multiple time zones, it was such a nightmare that seeing this, I wanted to defeat it immediately!

@kum-deepak
Copy link
Collaborator Author

I am actually in Stockholm for today 😄

gordonwoodhull added a commit that referenced this issue Apr 19, 2018
these may be harmless but we should consistently use UTC in all tests

for #1420
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants