SigninInteractionControllerTestCase is flaking every other test run |
|||||||
Issue descriptionTest run shows lots of OnOAuthError and Failed to get UserInfo for foo2ID
,
Jun 20 2018
This is blocking some CLs from landing, flakily, moving to p0
,
Jun 20 2018
The error message doesn't to be the real issue. It looks like there is an issue to enable-disable-enable sync too fast. https://paste.googleplex.com/5338444469370880
,
Jun 20 2018
,
Jun 20 2018
,
Jun 20 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fb745f666279d114ce0239d4a66520ab318c43c9 commit fb745f666279d114ce0239d4a66520ab318c43c9 Author: Jérôme Lebel <jlebel@chromium.org> Date: Wed Jun 20 15:03:49 2018 [iOS] Disabling flaky sync tests Disabling EarlGrey tests from SigninInteractionControllerTestCase: + testSignInSwitchAccountsAndKeepDataSeparate + testSignInSwitchAccountsAndImportData + testSignInSwitchManagedAccount Those tests seem to trigger an issue in the sync code when doing enable disable enable too fast. Bug: 854446 Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: I76d10d272297e91650dbe91ef7acee500eac6c75 Reviewed-on: https://chromium-review.googlesource.com/1107881 Reviewed-by: Justin Cohen <justincohen@chromium.org> Commit-Queue: Jérôme Lebel <jlebel@chromium.org> Cr-Commit-Position: refs/heads/master@{#568843} [modify] https://crrev.com/fb745f666279d114ce0239d4a66520ab318c43c9/ios/chrome/browser/ui/signin_interaction/signin_interaction_controller_egtest.mm
,
Jun 20 2018
Adjusting to P1 after affected tests have been disabled. My hypothesis is that ModelTypeController ignores stop events while sync is starting, which is reproduced in tests due to the fast enable-disable-enable sequence. Although unlikely, if this conditions is met, it could involve leaking sync metadata after sync has been disabled (or for some datatypes, leaking data). I'll confirm this and file a separate bug for that.
,
Jun 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a2f928a4359865bb5a040916a43201870d42176f commit a2f928a4359865bb5a040916a43201870d42176f Author: Mikel Astiz <mastiz@chromium.org> Date: Thu Jun 21 12:18:29 2018 Add DVLOG traces to ModelTypeController They are motivated by an ongoing investigation for test flakiness on iOS, with the suspicion that some of the codepaths (namely, where calls to OnSyncStopping() could be dropped) can cause issues (e.g. sync metadata being leaked). Bug: 854446 Change-Id: I6fe7da40c36d00e01afa176334f59f9dc66a74e1 Reviewed-on: https://chromium-review.googlesource.com/1108200 Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org> Commit-Queue: Mikel Astiz <mastiz@chromium.org> Cr-Commit-Position: refs/heads/master@{#569225} [modify] https://crrev.com/a2f928a4359865bb5a040916a43201870d42176f/components/sync/driver/model_type_controller.cc
,
Jun 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/06ca703c06156bed1d5f54c4aeaa58497f527346 commit 06ca703c06156bed1d5f54c4aeaa58497f527346 Author: Mikel Astiz <mastiz@chromium.org> Date: Thu Jun 21 15:32:44 2018 Improve sync enable-disable integration tests We refactor and extend the tests to: 1. Add support for USS types (which requires migrating the tests from reading the directory directly). 2. Capture more complex sequences, combining multiple transitions. 3. Add disabled tests that are under investigation, due to flakiness reported on iOS integration tests the exercise analogous transitions. Bug: 854446 Change-Id: Ic6aa9294086a8511255a084af08112f3455f078b Reviewed-on: https://chromium-review.googlesource.com/1110118 Reviewed-by: Marc Treib <treib@chromium.org> Commit-Queue: Mikel Astiz <mastiz@chromium.org> Cr-Commit-Position: refs/heads/master@{#569269} [modify] https://crrev.com/06ca703c06156bed1d5f54c4aeaa58497f527346/chrome/browser/sync/test/integration/enable_disable_test.cc
,
Jun 22 2018
,
Jun 25 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b4d3bf77f27e72b4448214e10205eb5216889f58 commit b4d3bf77f27e72b4448214e10205eb5216889f58 Author: Mikel Astiz <mastiz@chromium.org> Date: Mon Jun 25 14:55:08 2018 Propagate sync activation request to bridges This struct includes authenticated_account_id, which several bridges are interested in. This is safer and simpler to use compared to reading from side channels, and prevents a runtime dependency cycle across some factories. TBR=estade@chromium.org,rockot@chromium.org Bug: 850428, 834042 , 854446 Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: Ib17a4503ff9d284e1a5f4e222e7eeb95b08a024f Reviewed-on: https://chromium-review.googlesource.com/1111716 Commit-Queue: Mikel Astiz <mastiz@chromium.org> Reviewed-by: Mikel Astiz <mastiz@chromium.org> Reviewed-by: vitaliii <vitaliii@chromium.org> Reviewed-by: Marc Treib <treib@chromium.org> Reviewed-by: Christian Dullweber <dullweber@chromium.org> Cr-Commit-Position: refs/heads/master@{#570037} [modify] https://crrev.com/b4d3bf77f27e72b4448214e10205eb5216889f58/chrome/browser/consent_auditor/consent_auditor_factory.cc [modify] https://crrev.com/b4d3bf77f27e72b4448214e10205eb5216889f58/chrome/browser/extensions/api/sessions/sessions_apitest.cc [modify] https://crrev.com/b4d3bf77f27e72b4448214e10205eb5216889f58/chrome/browser/sync/user_event_service_factory.cc [modify] https://crrev.com/b4d3bf77f27e72b4448214e10205eb5216889f58/components/autofill/core/browser/webdata/autocomplete_sync_bridge_unittest.cc [modify] https://crrev.com/b4d3bf77f27e72b4448214e10205eb5216889f58/components/consent_auditor/consent_sync_bridge_impl.cc [modify] https://crrev.com/b4d3bf77f27e72b4448214e10205eb5216889f58/components/consent_auditor/consent_sync_bridge_impl.h [modify] https://crrev.com/b4d3bf77f27e72b4448214e10205eb5216889f58/components/consent_auditor/consent_sync_bridge_impl_unittest.cc [modify] https://crrev.com/b4d3bf77f27e72b4448214e10205eb5216889f58/components/sync/BUILD.gn [modify] https://crrev.com/b4d3bf77f27e72b4448214e10205eb5216889f58/components/sync/driver/model_type_controller.cc [rename] https://crrev.com/b4d3bf77f27e72b4448214e10205eb5216889f58/components/sync/model/data_type_activation_request.cc [rename] https://crrev.com/b4d3bf77f27e72b4448214e10205eb5216889f58/components/sync/model/data_type_activation_request.h [modify] https://crrev.com/b4d3bf77f27e72b4448214e10205eb5216889f58/components/sync/model/model_type_sync_bridge.cc [modify] https://crrev.com/b4d3bf77f27e72b4448214e10205eb5216889f58/components/sync/model/model_type_sync_bridge.h [modify] https://crrev.com/b4d3bf77f27e72b4448214e10205eb5216889f58/components/sync/model_impl/client_tag_based_model_type_processor.cc [modify] https://crrev.com/b4d3bf77f27e72b4448214e10205eb5216889f58/components/sync/model_impl/client_tag_based_model_type_processor.h [modify] https://crrev.com/b4d3bf77f27e72b4448214e10205eb5216889f58/components/sync/model_impl/client_tag_based_model_type_processor_unittest.cc [modify] https://crrev.com/b4d3bf77f27e72b4448214e10205eb5216889f58/components/sync/user_events/user_event_service_impl_unittest.cc [modify] https://crrev.com/b4d3bf77f27e72b4448214e10205eb5216889f58/components/sync/user_events/user_event_sync_bridge.cc [modify] https://crrev.com/b4d3bf77f27e72b4448214e10205eb5216889f58/components/sync/user_events/user_event_sync_bridge.h [modify] https://crrev.com/b4d3bf77f27e72b4448214e10205eb5216889f58/components/sync/user_events/user_event_sync_bridge_unittest.cc [modify] https://crrev.com/b4d3bf77f27e72b4448214e10205eb5216889f58/components/sync_sessions/session_sync_bridge.cc [modify] https://crrev.com/b4d3bf77f27e72b4448214e10205eb5216889f58/components/sync_sessions/session_sync_bridge.h [modify] https://crrev.com/b4d3bf77f27e72b4448214e10205eb5216889f58/components/sync_sessions/session_sync_bridge_unittest.cc [modify] https://crrev.com/b4d3bf77f27e72b4448214e10205eb5216889f58/ios/chrome/browser/consent_auditor/consent_auditor_factory.cc [modify] https://crrev.com/b4d3bf77f27e72b4448214e10205eb5216889f58/ios/chrome/browser/sync/ios_user_event_service_factory.cc
,
Jun 26 2018
Status update: I no longer repro failures on trunk (linux, newly added browser tests). I suspect there is still a minor issue (and I have a fix for it), but since I can't make tests fail, I'm not convinced it will repro on iOS either. Hence, I propose to reenable iOS tests and monitor flakes closely.
,
Jun 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/661767b98d165550e49b5a3f32ec3df80f48b6b2 commit 661767b98d165550e49b5a3f32ec3df80f48b6b2 Author: Mikel Astiz <mastiz@chromium.org> Date: Tue Jun 26 11:31:56 2018 Revert "[iOS] Disabling flaky sync tests" This reverts commit fb745f666279d114ce0239d4a66520ab318c43c9. Reason for revert: underlying issue no longer reproduces on other platforms after recent improvements, so optimistically assuming it was also fixed for iOS. Original change's description: > [iOS] Disabling flaky sync tests > > Disabling EarlGrey tests from SigninInteractionControllerTestCase: > + testSignInSwitchAccountsAndKeepDataSeparate > + testSignInSwitchAccountsAndImportData > + testSignInSwitchManagedAccount > > Those tests seem to trigger an issue in the sync code when doing enable > disable enable too fast. > > Bug: 854446 > Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet > Change-Id: I76d10d272297e91650dbe91ef7acee500eac6c75 > Reviewed-on: https://chromium-review.googlesource.com/1107881 > Reviewed-by: Justin Cohen <justincohen@chromium.org> > Commit-Queue: Jérôme Lebel <jlebel@chromium.org> > Cr-Commit-Position: refs/heads/master@{#568843} TBR=justincohen@chromium.org,jlebel@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 854446 Change-Id: Ic809289ce4662d099250668d6f23100e52de9587 Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet Reviewed-on: https://chromium-review.googlesource.com/1114719 Reviewed-by: Mikel Astiz <mastiz@chromium.org> Commit-Queue: Mikel Astiz <mastiz@chromium.org> Cr-Commit-Position: refs/heads/master@{#570374} [modify] https://crrev.com/661767b98d165550e49b5a3f32ec3df80f48b6b2/ios/chrome/browser/ui/signin_interaction/signin_interaction_controller_egtest.mm
,
Jun 26 2018
I'd appreciate if iOS folks could advice on what should be monitored. Taking a look at flakiness dashboards ([1]), the update frequency is very low. [1] https://test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&testType=ios_chrome_ui_egtests
,
Jun 27 2018
,
Jun 27 2018
Disabling tests again, given the observed flakes, in https://chromium-review.googlesource.com/c/chromium/src/+/1117578
,
Jun 27 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e6aa52bf65d037b1d119bd71c21eec17c5dcd341 commit e6aa52bf65d037b1d119bd71c21eec17c5dcd341 Author: Mikel Astiz <mastiz@chromium.org> Date: Wed Jun 27 20:33:35 2018 Reland "[iOS] Disabling flaky sync tests" This reverts commit 661767b98d165550e49b5a3f32ec3df80f48b6b2. Reason for revert: tests still flaky. Original change's description: > Revert "[iOS] Disabling flaky sync tests" > > This reverts commit fb745f666279d114ce0239d4a66520ab318c43c9. > > Reason for revert: underlying issue no longer reproduces on > other platforms after recent improvements, so optimistically > assuming it was also fixed for iOS. > > Original change's description: > > [iOS] Disabling flaky sync tests > > > > Disabling EarlGrey tests from SigninInteractionControllerTestCase: > > + testSignInSwitchAccountsAndKeepDataSeparate > > + testSignInSwitchAccountsAndImportData > > + testSignInSwitchManagedAccount > > > > Those tests seem to trigger an issue in the sync code when doing enable > > disable enable too fast. > > > > Bug: 854446 > > Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet > > Change-Id: I76d10d272297e91650dbe91ef7acee500eac6c75 > > Reviewed-on: https://chromium-review.googlesource.com/1107881 > > Reviewed-by: Justin Cohen <justincohen@chromium.org> > > Commit-Queue: Jérôme Lebel <jlebel@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#568843} > > TBR=justincohen@chromium.org,jlebel@chromium.org > > # Not skipping CQ checks because original CL landed > 1 day ago. > > Bug: 854446 > Change-Id: Ic809289ce4662d099250668d6f23100e52de9587 > Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet > Reviewed-on: https://chromium-review.googlesource.com/1114719 > Reviewed-by: Mikel Astiz <mastiz@chromium.org> > Commit-Queue: Mikel Astiz <mastiz@chromium.org> > Cr-Commit-Position: refs/heads/master@{#570374} TBR=justincohen@chromium.org,jlebel@chromium.org,mastiz@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 854446 Change-Id: I2a0a2e9d60988783fca93ef8313594d57b7ab906 Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet Reviewed-on: https://chromium-review.googlesource.com/1117578 Reviewed-by: Mikel Astiz <mastiz@chromium.org> Commit-Queue: Mikel Astiz <mastiz@chromium.org> Cr-Commit-Position: refs/heads/master@{#570888} [modify] https://crrev.com/e6aa52bf65d037b1d119bd71c21eec17c5dcd341/ios/chrome/browser/ui/signin_interaction/signin_interaction_controller_egtest.mm
,
Jun 28 2018
Issue 857186 has been merged into this issue.
,
Jul 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c8923dd5da2b94e56ccd7691f99ca6689c096348 commit c8923dd5da2b94e56ccd7691f99ca6689c096348 Author: Mikel Astiz <mastiz@chromium.org> Date: Wed Jul 04 15:07:00 2018 Revert "Reland "[iOS] Disabling flaky sync tests"" This reverts commit e6aa52bf65d037b1d119bd71c21eec17c5dcd341. Reason for revert: recent patches fix a related problem, in 23919ce6813da7b70e2d5cf488345f5f7f42fa1d. We haven't been able to confirm the fix being effective for the flakes observed in bots because they no longer repro locally, but it is definitely related so let's reenable the tests and monitor flakes. Original change's description: > Reland "[iOS] Disabling flaky sync tests" > > This reverts commit 661767b98d165550e49b5a3f32ec3df80f48b6b2. > > Reason for revert: tests still flaky. > > Original change's description: > > Revert "[iOS] Disabling flaky sync tests" > > > > This reverts commit fb745f666279d114ce0239d4a66520ab318c43c9. > > > > Reason for revert: underlying issue no longer reproduces on > > other platforms after recent improvements, so optimistically > > assuming it was also fixed for iOS. > > > > Original change's description: > > > [iOS] Disabling flaky sync tests > > > > > > Disabling EarlGrey tests from SigninInteractionControllerTestCase: > > > + testSignInSwitchAccountsAndKeepDataSeparate > > > + testSignInSwitchAccountsAndImportData > > > + testSignInSwitchManagedAccount > > > > > > Those tests seem to trigger an issue in the sync code when doing enable > > > disable enable too fast. > > > > > > Bug: 854446 > > > Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet > > > Change-Id: I76d10d272297e91650dbe91ef7acee500eac6c75 > > > Reviewed-on: https://chromium-review.googlesource.com/1107881 > > > Reviewed-by: Justin Cohen <justincohen@chromium.org> > > > Commit-Queue: Jérôme Lebel <jlebel@chromium.org> > > > Cr-Commit-Position: refs/heads/master@{#568843} > > > > TBR=justincohen@chromium.org,jlebel@chromium.org > > > > # Not skipping CQ checks because original CL landed > 1 day ago. > > > > Bug: 854446 > > Change-Id: Ic809289ce4662d099250668d6f23100e52de9587 > > Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet > > Reviewed-on: https://chromium-review.googlesource.com/1114719 > > Reviewed-by: Mikel Astiz <mastiz@chromium.org> > > Commit-Queue: Mikel Astiz <mastiz@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#570374} > > TBR=justincohen@chromium.org,jlebel@chromium.org,mastiz@chromium.org > > # Not skipping CQ checks because original CL landed > 1 day ago. > > Bug: 854446 > Change-Id: I2a0a2e9d60988783fca93ef8313594d57b7ab906 > Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet > Reviewed-on: https://chromium-review.googlesource.com/1117578 > Reviewed-by: Mikel Astiz <mastiz@chromium.org> > Commit-Queue: Mikel Astiz <mastiz@chromium.org> > Cr-Commit-Position: refs/heads/master@{#570888} TBR=justincohen@chromium.org,jlebel@chromium.org,mastiz@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 854446 Change-Id: I5874890bed60c173c35e49fe86933a06bd214120 Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet Reviewed-on: https://chromium-review.googlesource.com/1126260 Reviewed-by: Mikel Astiz <mastiz@chromium.org> Commit-Queue: Mikel Astiz <mastiz@chromium.org> Cr-Commit-Position: refs/heads/master@{#572570} [modify] https://crrev.com/c8923dd5da2b94e56ccd7691f99ca6689c096348/ios/chrome/browser/ui/signin_interaction/signin_interaction_controller_egtest.mm
,
Jul 13
No flakes in more than a week, so considering this finally fixed! |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by justincohen@chromium.org
, Jun 20 2018