CFTimeZoneCopySystem fails on iOS 10.0 simulator running on High Sierra |
|||||||||||||||||
Issue descriptionbase_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)
,
Nov 7 2017
Flaky failure continues today. If the test is running with Swarming, could there be a misconfigured device somewhere in the Swarming machine pool?
,
Nov 8 2017
,
Nov 8 2017
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?
,
Nov 8 2017
Removing Sheriff-Chromium now that the bug is assigned and being investigated.
,
Nov 10 2017
Ping; this is a random failure on trybots making life difficult. I reverted an unrelated change due to this.
,
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.
,
Nov 11 2017
Maybe we should disable on IOS until issue sorted out?
,
Nov 13 2017
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).
,
Nov 13 2017
Both success and fail test run on vm618-m1 Failing https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.mac%2Fios-simulator%2F25534%2F%2B%2Frecipes%2Fsteps%2Fbase_unittests__iPad_Air_2_iOS_10.0_%2F0%2Flogs%2Fswarming.summary%2F0 Success https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.mac%2Fios-simulator%2F25532%2F%2B%2Frecipes%2Fsteps%2Fbase_unittests__iPad_Air_2_iOS_10.0_%2F0%2Flogs%2Fswarming.summary%2F0
,
Nov 13 2017
No visible pattern on the failure. Should we disable it?
,
Nov 13 2017
If we disable TimeTest.TimeT on ios-simulator, is it still covered on iOS?
,
Nov 13 2017
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
,
Nov 13 2017
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.
,
Nov 14 2017
Assigning to Olivier as sheriff.
,
Nov 14 2017
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.
,
Nov 14 2017
,
Nov 15 2017
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 :).
,
Nov 15 2017
Could we just check the OS version in the test?
,
Nov 15 2017
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
,
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
,
Nov 15 2017
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.
,
Nov 15 2017
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).
,
Nov 15 2017
I should get a machine running High Sierra today/tomorrow and I'll be able to investigate. Assigning to self.
,
Nov 15 2017
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)
,
Nov 15 2017
By 10.13, you mean High Sierra, right? AFAIK, there is no iOS 10.13 version. This is macOS version of base_unittests?
,
Nov 15 2017
Yes.
,
Nov 15 2017
That sounds like a separate issue. I'll file a new bug for that.
,
Nov 15 2017
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.
,
Nov 15 2017
It's a different test. One that has been newly added vs the other test that was failing before the recent CL.
,
Nov 15 2017
Oh yes, you're right. Sorry for the noise.
,
Nov 15 2017
,
Nov 15 2017
,
Nov 16 2017
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.
,
Nov 16 2017
Issue 783319 has been merged into this issue.
,
Nov 16 2017
Looks like the patch in #21 failed to disable it, see recent failure: https://build.chromium.org/p/chromium.mac/builders/ios-simulator/builds/25741 Uploaded https://chromium-review.googlesource.com/c/chromium/src/+/774993
,
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
,
Nov 20 2017
,
Nov 20 2017
huangml: Does this relate to your CL here: https://chromium-review.googlesource.com/c/chromium/src/+/775792 ?
,
Nov 20 2017
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.
,
Nov 21 2017
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.
,
Nov 21 2017
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.
,
Nov 21 2017
Reported the issue with CFTimeZoneCopySystem to Apple as radar://35657328
,
Nov 21 2017
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.
,
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
,
Nov 24 2017
Reported the issue with CFTimeZoneCopySystem to Apple as radar://35657328
,
Dec 20 2017
Apple responded: Engineering has provided the following information regarding this issue: We believe this to be fixed in the iOS 11 simulator.
,
Dec 20 2017
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).
,
Jan 5 2018
,
Apr 20 2018
|
|||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||
Comment 1 by rohitrao@chromium.org
, Nov 7 2017Owner: sdefresne@chromium.org