New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 782033 link

Starred by 8 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS , Mac
Pri: 3
Type: Bug



Sign in to add a comment

CFTimeZoneCopySystem fails on iOS 10.0 simulator running on High Sierra

Project Member Reported by sky@chromium.org, Nov 7 2017

Issue description

base_unittests (iPad Air 2 iOS 10.0) failing on chromium.mac/ios-simulator

Builders failed on: 
- ios-simulator: 
  https://build.chromium.org/p/chromium.mac/builders/ios-simulator


It looks like TimeTest.TimeT has been flaky for a while. Looking at the history, it isn't always the same config either. Meaning some times ipad air 2 ios 10.0, some times 11.0... Here's sample output:
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from TimeTest
[ RUN      ] TimeTest.TimeT
../../base/time/time_unittest.cc:134: Failure
      Expected: tms.tm_hour
      Which is: 15
To be equal to: exploded.hour
      Which is: 23
[  FAILED  ] TimeTest.TimeT (1 ms)
 
Cc: olivierrobin@chromium.org
Owner: sdefresne@chromium.org
Sylvain and Olivier, is this something you could take a look at?

I'm not sure what changed recently to cause this test to start failing.  https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=base_unittests&tests=TimeTest.TimeT

I do notice that the error is 8 hours, which suspiciously is the difference between UTC and PST.
Flaky failure continues today. If the test is running with Swarming, could there be a misconfigured device somewhere in the Swarming machine pool?
Status: Assigned (was: Available)
There is nothing obvious here.
Failure is always on iPad Air 2, iOS 10.0
Passing and failing tests are on the same machine: slavename:vm618-m1
The documentation of CFTimeZoneCopySystem states that it can return GMT if timezon cannot be determined.
I guess it may be what happens here?
Labels: -Sheriff-Chromium
Removing Sheriff-Chromium now that the bug is assigned and being investigated.

Comment 6 by a...@chromium.org, Nov 10 2017

Ping; this is a random failure on trybots making life difficult. I reverted an unrelated change due to this.

Comment 7 by mmenke@chromium.org, Nov 11 2017

Is this a timezone issue?  Another failure has:

../../base/time/time_unittest.cc:134: Failure
      Expected: tms.tm_hour
      Which is: 14
To be equal to: exploded.hour
      Which is: 22

Times are also 8 hours off, but times are different.

Comment 8 by sky@chromium.org, Nov 11 2017

Maybe we should disable on IOS until issue sorted out?
Owner: gambard@chromium.org
Assigning to sherriff (gambard@). We suspect this is an misconfigured bot with regard to timezone. Could you investigate whether this consistently happens on a single bot (or a subset of bots) to confirm/infirm the theory? If it is confirmed to be a misconfiguration, then we'll have to ask troopers to fix the configuration of the bot (or to remove it from the pool).
Cc: sdefresne@chromium.org
No visible pattern on the failure.
Should we disable it?
If we disable TimeTest.TimeT on ios-simulator, is it still covered on iOS?
Re: #10, you need to look at the swarmed bot and not the vm.
I had the test fail for me here https://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/builds/338127, which was on build10-m9, which seems to always fail it:
https://chromium-swarm.appspot.com/bot?id=build10-m9&sort_stats=total%3Adesc

The failing test from #10 was from build35-m9, which also seems to always fail it:
https://chromium-swarm.appspot.com/bot?id=build35-m9&sort_stats=total%3Adesc
I've looked at the succeeding bot from #10, build617-m4.
https://chromium-swarm.appspot.com/bot?id=build617-m4&sort_stats=total%3Adesc

I think the difference might be that it succeeds on Mac-10.12.6 and fails on Mac-10.13.
Owner: olivierrobin@chromium.org
Assigning to Olivier as sheriff.
Would it be possible to restrict the bots running iOS tests to macOS 10.12?

We cannot test as macOS 10.13 as this is not deployed on corp yet.

Comment 17 by a...@chromium.org, Nov 14 2017

Cc: a...@chromium.org
 Issue 784902  has been merged into this issue.
Cc: sergeybe...@chromium.org
it doesn't look like we can easily restrict just a single test suite to run on 10.12. The iOS recipes look like they don't support specifying swarming dimensions per-test (which is a problem for other things as well). And, it looks like we probably don't have the capacity to just switch all of the tests to only run on 10.12.

In the short term, I think we need to disable the test, which I will proceed with now :).
Could we just check the OS version in the test?
Cc: mark@chromium.org
Possibly. I welcome someone to make that change in a follow-up CL :).

This CL disables it for iOS across the board, which seems fine for now: https://crrev.com/c/770451
Project Member

Comment 21 by bugdroid1@chromium.org, Nov 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1ecc28e5f3364860508df3960924be66358b14a5

commit 1ecc28e5f3364860508df3960924be66358b14a5
Author: Dirk Pranke <dpranke@chromium.org>
Date: Wed Nov 15 02:35:57 2017

Disable flaky TimeTest.TimeT base_unittests test on iOS.

It looks like this is failing in the iPaid Air 2/iOS 10.0
config, at least on 10.13, but we don't have a good way
of restricting the failure to just that config, so this
CL disables the test across the board on iOS.

TBR=olivierrobin@chromium.org, sergeyberezin@chromium.org, mark@chromium.org
BUG=782033

Change-Id: I3fae36733f226bc56e5161bce5b95028dcb958cd
Reviewed-on: https://chromium-review.googlesource.com/770451
Commit-Queue: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516567}
[modify] https://crrev.com/1ecc28e5f3364860508df3960924be66358b14a5/base/time/time_unittest.cc

Comment 22 by mark@chromium.org, Nov 15 2017

Cc: m...@chromium.org
Labels: -Pri-2 M-64 OS-Mac Pri-1
Summary: base_unittests (iPad Air 2 iOS 10.0) failing on chromium.mac/ios-simulator (and macOS 10.13?) (was: base_unittests (iPad Air 2 iOS 10.0) failing on chromium.mac/ios-simulator)
miu wrote in https://chromium-review.googlesource.com/c/chromium/src/+/770451#message-e95182b066b97cc30db05ced78902781c3a82979, regarding disabling the test on iOS:

> IMHO, this is too much of a "big stick" approach for sensitive platform code (e.g., a > broken Time might expose security vulnerablities). […]

https://chromium-review.googlesource.com/c/chromium/src/+/770451#message-7ecf5ece4039e1e4d01b2a68df32036490b1886c:

> I agree that this is crucial enough that maintaining this for any appreciable amount
> of time would be irresponsible on our part. Accordingly, I’m increasing the P of
> https://crbug.com/782033, and setting an M.
It looks like this is a bug in the interaction of the simulator and the new version of macOS. Maybe we should report this as a radar:// to Apple (someone with access to High Sierra needs to do this, no-one in the Chrome on iOS in Paris has access to this version of the OS).
Owner: sdefresne@chromium.org
I should get a machine running High Sierra today/tomorrow and I'll be able to investigate. Assigning to self.

Comment 25 by mark@chromium.org, Nov 15 2017

Cc: asvitk...@chromium.org
I also see this on 10.13, native, not on a simulated iOS device:

[ RUN      ] TimeTest.FromExploded_MinMax
../../base/time/time_unittest.cc:660: Failure
Value of: Time::FromUTCExploded(exploded, &parsed_time)
  Actual: true
Expected: false
../../base/time/time_unittest.cc:661: Failure
Value of: parsed_time.is_null()
  Actual: false
Expected: true
[  FAILED  ] TimeTest.FromExploded_MinMax (0 ms)
By 10.13, you mean High Sierra, right? AFAIK, there is no iOS 10.13 version. This is macOS version of base_unittests?

Comment 27 by mark@chromium.org, Nov 15 2017

Yes.
That sounds like a separate issue. I'll file a new bug for that.
I'm not sure that it is a separate issue. IIRC, the iOS simulator does invoke the host OS implementation of many core services (to avoid having to reimplement them). So it would make sense that the simulator would exhibit the same behavior as the host OS.
It's a different test. One that has been newly added vs the other test that was failing before the recent CL.
Oh yes, you're right. Sorry for the noise.
Cc: huangml@chromium.org baxley@chromium.org
 Issue 785185  has been merged into this issue.
I copied base_unittests.app to my personal 10.13 mac and verified that TimeT fails there on simulator.  When I run it locally (on the East Coast), the actual hour is off by 5, which makes me pretty confident that this is a timezone issue.

I'm guessing that the problem is somewhere in LocalExplode() in time_mac.cc, but I don't know what's going wrong.

@sdefresne should be able to investigate more tomorrow.
 Issue 783319  has been merged into this issue.
Project Member

Comment 37 by bugdroid1@chromium.org, Nov 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d9285a5a764b86a19cc9da710ab905df44db0c15

commit d9285a5a764b86a19cc9da710ab905df44db0c15
Author: Robert Flack <flackr@chromium.org>
Date: Thu Nov 16 21:25:03 2017

Use MAYBE_TimeT macro to disable flaky TimeTest.TimeT base_unittests test on iOS.

This is a followup to https://crrev.com/c/770451 which added MAYBE_TimeT but didn't use it to disable the test.

TBR=mark@chromium.org

Bug: 782033
Change-Id: Ic6b4d319fcc572c78b01a8642ea415e77dc1a2eb
Reviewed-on: https://chromium-review.googlesource.com/774993
Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517196}
[modify] https://crrev.com/d9285a5a764b86a19cc9da710ab905df44db0c15/base/time/time_unittest.cc

Comment 38 by pkl@chromium.org, Nov 20 2017

Components: Tests

Comment 39 by pkl@chromium.org, Nov 20 2017

huangml: Does this relate to your CL here: https://chromium-review.googlesource.com/c/chromium/src/+/775792 ?
CL https://chromium-review.googlesource.com/c/chromium/src/+/775792 makes base_unittest run on mac10.12 bots only.  Now we can re-enable the TimeT test to have the coverage on CQ before this bug is fixed.
I've debugged this, and it appears that when the host mac is running macOS 10.13 (aka High Sierra) and simulating iOS 10.0, then the call to CFTimeZoneCopySystem() fails to return the system timezone and instead returns the "GMT" timezone.

So, we have the following matrix, corresponding to CFTimeZoneCopySystem() behaviour:

             ,---------------------------.
             | macOS 10.12 | macOS 10.13 |
+------------+-------------+-------------+
| iOS 10.0   |   Local     |   GMT       |
+------------+-------------+-------------+
| iOS 11.0   |   Local     |   Local     |
+------------+-------------+-------------+

According to the documentation for CFTimeZoneCopySystem, returning GMT happens when the function is unable to determine the system timezone:

> A time zone representing the time zone currently used by the system, or
> the GMT time zone if the current zone cannot be determined.

https://developer.apple.com/documentation/corefoundation/1543660-cftimezonecopysystem?language=objc

I think this is a bug in Apple software that should be reported to them. We should be able to re-enable the test to run on iOS 11.0 (I don't think it is possible to detect the version of the OS running on the host from within the app running in the simulator) and one devices.

Another fix could be to change the test to use gmttime instead of localtime to do the comparison, but this would requires agreement from //base OWNERS.
I created two CLs implementing the two alternatives:

- https://chromium-review.googlesource.com/c/chromium/src/+/781464 disable the test on iOS 10.0 simulator until the function is fixed by Apple

- https://chromium-review.googlesource.com/c/chromium/src/+/781860 convert the test to use GMT methods (gmtime_? & UTCExplode) to avoid calling the function

I personally prefer the second CL as it allows us to run the test on all configurations, but it slightly change what the test exercise (and alternative would be to duplicate the test, and disable the one using local time on iOS 10.0 simulator).

Sending both CLs to the same OWNERS of //base.

Summary: CFTimeZoneCopySystem fails on iOS 10.0 simulator running on High Sierra (was: base_unittests (iPad Air 2 iOS 10.0) failing on chromium.mac/ios-simulator (and macOS 10.13?))
Reported the issue with CFTimeZoneCopySystem to Apple as radar://35657328
If we change the test to use GMT methods, then we'll be losing test coverage for the original issue that this uncovered. I don't think that's a good outcome.

Duplicating the tests and having one for GMT and one for local timezone sounds like a better approach to me.
Project Member

Comment 45 by bugdroid1@chromium.org, Nov 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2eaeaf84f78b2d7957fcdf2131c8f17a646199d1

commit 2eaeaf84f78b2d7957fcdf2131c8f17a646199d1
Author: Sylvain Defresne <sdefresne@chromium.org>
Date: Fri Nov 24 14:55:48 2017

Selectively disable TimeTest.TimeT.

CFTimeZoneCopySystem fails to determine the system timezone when
simulating iOS 10.0 on an host running High Sierra. Disable the
test on iOS if running on 10.0 simulator (as it is not possible
to check the version of the host).

Add another variant of the test using UTC time to test that the
logic is correct (this version is unaffected by the issue with
CFTimeZoneCopySystem).

Extract tests that were independent of the timezone to another
test case.

Bug: 782033
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I67821c13c3588a40e060b11667d90c4ccab7b54f
Reviewed-on: https://chromium-review.googlesource.com/787476
Reviewed-by: Yuri Wiitala <miu@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519112}
[modify] https://crrev.com/2eaeaf84f78b2d7957fcdf2131c8f17a646199d1/base/time/time_unittest.cc
[modify] https://crrev.com/2eaeaf84f78b2d7957fcdf2131c8f17a646199d1/ios/build/bots/tests/screen_size_dependent_tests.json

Status: ExternalDependency (was: Assigned)
Reported the issue with CFTimeZoneCopySystem to Apple as radar://35657328

Apple responded:
Engineering has provided the following information regarding this issue:
We believe this to be fixed in the iOS 11 simulator.
Labels: -Pri-1 Pri-3
This is not really helpful given that the issue is on iOS 10 simulator :-/ and the issue was reported as working correctly on iOS 11 simulator.

Reducing priority as we have a workaround. Keeping the bug open so that we can remove the workaround if/when we drop support for iOS 10 (or 32-bit devices whatever happens first, as we'll then be able to use the same API as mac).
Labels: -M-64 -Filed-Via-SoM
Owner: ----
Status: Available (was: ExternalDependency)

Sign in to add a comment