Skip to content

Ensure that tests are always executed in a proper time zone #2551

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

Merged
merged 1 commit into from
Oct 9, 2018

Conversation

akosthekiss
Copy link
Member

The America/Los_Angeles time zone is already enforced for Travis CI
jobs but that doesn't guarantee the correctness of locally executed
tests. So, this patch moves the setting of the TZ environment
variable from .travis.yml to run-tests.py.

JerryScript-DCO-1.0-Signed-off-by: Akos Kiss akiss@inf.u-szeged.hu

@akosthekiss akosthekiss added infrastructure Related to GH Actions or the tested targets tools Related to the tooling scripts test Related to testing labels Oct 4, 2018
@crazy2be
Copy link
Contributor

crazy2be commented Oct 4, 2018

Just curious - it looks like several of the jerryscript date tests (python tools/run-tests.py --jerry-tests) also rely on being in a particular timezone. Running them in my local timezone, all 4 date related tests fail. I was hoping this patch would fix that, but I still have the same 4 tests failing. Changing my computer timezone to California time, the same 4 tests still fail. Changing my computer timezone to GMT, I only fail the date-setters.js test. However, presumably there is some configuration that causes these tests to all pass, since they appear to pass on travis-ci.

@akosthekiss
Copy link
Member Author

@crazy2be What's your setup? I just tried TZ='Europe/London' tools/run-tests.py --jerry-tests on macOS 10.13.6 (Darwin 17.7.0 x86_64) and on Ubuntu 18.04.1 LTS (Linux 4.15.0 x86_64), and got all tests passing.

And BTW, did the test262 tests pass for you with this patch?

@crazy2be
Copy link
Contributor

crazy2be commented Oct 5, 2018

Hmm, interesting. All tests appear to pass for me as well on master with a manually specified timezone like that, but date_setters.js definitely fails for me on my timezone fixing branch (#2540). I'm on OSX 10.13.2 (Darwin 17.3.0 x86_64). It isn't surprising this test would start failing, but I'm surprised it's not failing on travis-ci...

The relevant thought I had with regards to this PR is that we should make the test-runner set the correct timezone for the jerry-tests, since we are now doing it for the test262 tests (as of this PR!). Then people in other timezones (like me!) can just run the tests without expecting failures, or having to remember the correct timezone for each suite of tests.

@akosthekiss
Copy link
Member Author

I think it should be possible to lock the TZ for the other tests as well. I'll update this patch later tonight. I'll try to set TZ to Europe/Budapest. One is allowed to be just a bit patriotic, right? :)

@crazy2be
Copy link
Contributor

crazy2be commented Oct 5, 2018

Ok, I figured out the date-setters.js problem. The following command makes all tests execute successfully (I can even uncomment the dst ones, and they work too!):

TZ='UTC' tools/run-tests.py --jerry-tests

However, now that we have proper timezone/dst support, any other timezone causes failures. This makes sense, since the part that is failing is Date.setFullYear, which sets the year (in local time, as per the spec), and then out tests assert that the returned ms value (which is in UTC time) is a particular value:

assert (d.setFullYear(0, 0) == -62167219200000);

This shouldn't be expected to work in any timezone other than the one used to retrieve the UTC ms value, which clearly was UTC, given

assert (d.setUTCFullYear(0, 0) == -62167219200000);

Thus, we can (and should!) make sure these tests are always run in a particular timezone. However, if that timezone is anything other than UTC, it will require some additional adjustments to the tests in #2540 in order to retrieve the correct values for the given timezone.

@crazy2be
Copy link
Contributor

crazy2be commented Oct 5, 2018

I realize I never answered your second question. Yes, this patch does in fact make it so that the test262 tests pass on my system, without having to change my timezone manually.

ch15/15.9/15.9.3/S15.9.3.1_A5_T1 passed in non-strict mode
ch15/15.9/15.9.3/S15.9.3.1_A5_T2 passed in non-strict mode
ch15/15.9/15.9.3/S15.9.3.1_A5_T3 passed in non-strict mode
ch15/15.9/15.9.3/S15.9.3.1_A5_T4 passed in non-strict mode
ch15/15.9/15.9.3/S15.9.3.1_A5_T5 passed in non-strict mode
ch15/15.9/15.9.3/S15.9.3.1_A5_T6 passed in non-strict mode
=== Summary ===
 - Ran 11552 tests
 - All tests succeeded

With just the command line

tools/run-tests.py --test262

Awesome!

@akosthekiss
Copy link
Member Author

Thanks for the analysis (UTC it will be then) and thanks for the feedback.

The America/Los_Angeles time zone is already enforced for test262
Travis CI jobs but that doesn't guarantee the correctness of
locally executed tests. So, this patch moves the setting of the
`TZ` environment variable from `.travis.yml` to `run-tests.py`.

The date-related tests in the jerry test suite also rely on a time
zone (UTC). Thus, `TZ` is forced for those tests, too.

JerryScript-DCO-1.0-Signed-off-by: Akos Kiss akiss@inf.u-szeged.hu
@akosthekiss akosthekiss changed the title Ensure that test262 tests are always executed in the proper time zone Ensure that tests are always executed in a proper time zone Oct 7, 2018
@akosthekiss
Copy link
Member Author

PR updated. Proper time zone is enforced for both test262 and jerry tests.

Copy link
Member

@zherczeg zherczeg left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@LaszloLango LaszloLango left a comment

Choose a reason for hiding this comment

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

LGTM

@LaszloLango LaszloLango merged commit afe2a80 into jerryscript-project:master Oct 9, 2018
@akosthekiss akosthekiss deleted the run-test262-tz branch October 9, 2018 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Related to GH Actions or the tested targets test Related to testing tools Related to the tooling scripts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants