Bug 186612 – Ensure the tests are run with US/Pacific time zone

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Carlos Alberto Lopez Perez]

It seems that when you run layout tests on a machine not configured with US/Pacific timezone some tests fail, like:

Regressions: Unexpected text-only failures (6)
  storage/indexeddb/modern/date-basic-private.html [ Failure ]
  storage/indexeddb/modern/date-basic.html [ Failure ]
  storage/indexeddb/modern/get-keyrange-private.html [ Failure ]
  storage/indexeddb/modern/get-keyrange.html [ Failure ]
  storage/indexeddb/modern/idbobjectstore-delete-1-private.html [ Failure ]
  storage/indexeddb/modern/idbobjectstore-delete-1.html [ Failure ]


In order to fix this and avoid potential new issues related to this I propose to unconditionally ensure the tests are always run with an US/Pacific timezone.

The JavaScriptCore tests already do this after https://trac.webkit.org/changeset/173116

[Carlos Alberto Lopez Perez]

Created attachment 342713 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 342713 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=342713&action=review

> Tools/ChangeLog:8
> +        Unconditionally set TZ environment variable to US/Pacific for the test environment.

I don't think that we should do this. Tests need to run inside the browser too, so making them depend on a specific time zone would result in not making them resilient. Also, bug 137473 alludes to us getting some valuable testing from not forcing the time zone.

[Carlos Alberto Lopez Perez]

(In reply to Alexey Proskuryakov from comment #2)
> Comment on attachment 342713 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=342713&action=review
> 
> > Tools/ChangeLog:8
> > +        Unconditionally set TZ environment variable to US/Pacific for the test environment.
> 
> I don't think that we should do this. Tests need to run inside the browser
> too, so making them depend on a specific time zone would result in not
> making them resilient.

Well... the status quo is that all the bots (at least the ones that Igalia maintain) run with an US/Pacific timezone by default to avoid this and other issues.

Also a big part of the WebKit contributors live on that timezone.

So it will happen tests will be created with expectations of running on that timezone (not on purpose, but it will happen).

And the only way of noticing this will be that a contributor not living in that timezone get bitten by this and report a bug (like I'm doing)


> Also, bug 137473 alludes to us getting some valuable
> testing from not forcing the time zone.

That is already 4 years old.

In any case this patch doesn't force the timezone.

It just changes the default from whatever the system has to US/Pacific.

[EWS Watchlist]

Comment on attachment 342713 [details]
Patch

Attachment 342713 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/8184693

New failing tests:
http/tests/misc/last-modified-parsing.html

[EWS Watchlist]

Created attachment 342762 [details]
Archive of layout-test-results from ews200 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews200  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Alexey Proskuryakov]

> Also a big part of the WebKit contributors live on that timezone.

That's... weird. I certainly didn't on PST before moving to Apple, so what changed? How many tests are failing?

Even bots switch between PST and DST as appropriate.

[Adrian Perez]

(In reply to Alexey Proskuryakov from comment #6)
> > Also a big part of the WebKit contributors live on that timezone.
> 
> That's... weird. I certainly didn't on PST before moving to Apple, so what
> changed? How many tests are failing?
> 
> Even bots switch between PST and DST as appropriate.

The issue is not PST vs. PST, but US/Pacific vs. any other timezone.

I think ideally it would be indeed desirable that tests would run
independently of the configured timezone, but certainly we're not
quite there yet, so the suggested patch looks like a good stopgap
measure to me.

(And even if tests in general ran independently of the timezone,
it looks to me for some of them it would be still needed to set
the a known timezone for a certain test, because it still seems
like a good idea that reference test results are fixed.)

[Alexey Proskuryakov]

> The issue is not PST vs. PST, but US/Pacific vs. any other timezone.

DST and PST are different time zones too though.

> I think ideally it would be indeed desirable that tests would run
> independently of the configured timezone, but certainly we're not
> quite there yet, so the suggested patch looks like a good stopgap
> measure to me.

You are saying that moving in the opposite direction from ideal to the ideal is a stopgap measure. I don't see how that would help improve things.

[Alexey Proskuryakov]

s/to the ideal//

[Adrian Perez]

(In reply to Alexey Proskuryakov from comment #8)
> > The issue is not PST vs. PST, but US/Pacific vs. any other timezone.
> 
> DST and PST are different time zones too though.

Ouch, it seems I was very sleepy when I wrote this yesterday... In
my head I was thinking “DST vs. non-DST” (Daylight Savings Time).
Sorry, please forget about what I wrote there.

[Alexey Proskuryakov]

Sorry, I did mean PDT vs. PST indeed.

[Sam Sneddon [:gsnedders]]

These tests are _still_ failing for non-Pacific timezones. Clearly, things _aren't_ improving and those of us outside of Pacific-time have to deal with extra noise in our test results on a daily basis. By not even working around this, it's hurting almost all non-Apple contributors to WebKit, which really isn't great.

We should do one of two things here:

 * follow the approach already taken by JavaScriptCore to make these deterministic and leave fixing the tests for all timezones as future work (and preferably set up a bot to test things in a non-California-centric configuration!), or

 * mark these tests as flaky, given that's what they are.

[Carlos Alberto Lopez Perez]

I still think forcing a US/Pacific timezone before starting the tests is the most pragmatic approach. The patch attached here should do that.

[Michael Catanzaro]

Comment on attachment 342713 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=342713&action=review

> Tools/Scripts/webkitpy/port/base.py:948
> +        # Some tests fail if the time zone is not set to US/Pacific (<https://webkit.org/b/186612>)
> +        clean_env['TZ'] = 'US/Pacific'

Alexey's concern is reasonable. I would add a FIXME: to your comment to make it clear that this is a concession rather than desired behavior.

Of course it would be nicer if tests didn't depend on timezone, but fixing every test doesn't seem like a priority. I've never considered running tests locally to be a reasonable option due to very high number of local failures that don't occur on bots, and anything to reduce that seems like a good step towards making it practical to run tests locally.

[Alexey Proskuryakov]

I'm no longer strongly opposed to this change. Although I still think that tests need to work properly in any time zone, they shouldn't expect PST/DST.

[Carlos Alberto Lopez Perez]

(In reply to Alexey Proskuryakov from comment #15)
> I'm no longer strongly opposed to this change. Although I still think that
> tests need to work properly in any time zone, they shouldn't expect PST/DST.

I agree that tests should work in any timezone.

This is not a solution to fix those tests. Its only a workaround to make the issue less annoying to those not living in that timezone meanwhile the tests are not fixed.

I will add a FIXME comment pointing to this bug so eventually this can be removed if those tests are fixed.

[Carlos Alberto Lopez Perez]

Created attachment 411521 [details]
Patch

patch for landing, test EWS first

[EWS]

Committed r268581: <https://trac.webkit.org/changeset/268581>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 411521 [details].

[Radar WebKit Bug Importer]

<rdar://problem/70374961>