cronet_tests fail under component builds |
||
Issue descriptioncronet_tests is intended to load the Cronet native library and verify the APIs it provides. In component builds the test executable and library both end up sharing the same copies of components including //base and //net, causing failures due to unexpected sharing of process-global state, including: - TaskManager clashes if any test attempts to use ScopedTaskEnvironment, via shared //base. - AtExitManager clashes, via shared //base. Addressing this is made more complex by the difference in lifetime expectations - ScopedTaskEnvironment is typically created per-test, whereas the library's TaskScheduler instances is created during the first call to Initialize(), and persists from then on - the process cannot replace the TaskScheduler with a ScopedTaskEnvironment version (e.g. to use MOCK_TIME) without resetting the library in some fashion. Ideally we should statically-link the Cronet library to its dependencies even in component builds. Alternatively we could have the component build of the library have different initialization paths for component (test-only) vs non-component builds, though that may still be difficult to arrange. For now, we'll disable cronet_tests on our Debug/component bots.
,
Feb 27 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0e38a719e04df5b58886bb6c17ad9d7e5f458628 commit 0e38a719e04df5b58886bb6c17ad9d7e5f458628 Author: Wez <wez@chromium.org> Date: Tue Feb 27 03:28:57 2018 Disable cronet_tests on Fuchsia (dbg). These tests expect that library-global and test-process-global state is independent, which is not the case in component builds where critical subcomponents such as //base and //net are shared. TBR: jbudorick Bug: 816705 , 808075 Change-Id: I1ca3f670d472d588b3fabc584b6eaa7ce1f08de2 Reviewed-on: https://chromium-review.googlesource.com/938680 Reviewed-by: Wez <wez@chromium.org> Reviewed-by: Scott Graham <scottmg@chromium.org> Commit-Queue: Wez <wez@chromium.org> Cr-Commit-Position: refs/heads/master@{#539390} [modify] https://crrev.com/0e38a719e04df5b58886bb6c17ad9d7e5f458628/testing/buildbot/chromium.fyi.json [modify] https://crrev.com/0e38a719e04df5b58886bb6c17ad9d7e5f458628/testing/buildbot/test_suite_exceptions.pyl
,
Apr 6 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8fa7ecedfa0a5de4a27d93979d330b91e531b00c commit 8fa7ecedfa0a5de4a27d93979d330b91e531b00c Author: Wez <wez@chromium.org> Date: Fri Apr 06 04:52:24 2018 Enable Cronet tests on desktop platforms. - Only initialize global state (e.g. AtExitManager) in non-Debug builds, so that our component- build Debug bots can run cronet_tests. - Fix memory leaks in a number of unit-tests. - Disable some StaleHostResolver tests under ASAN, since they have leaks that need resolving. Bug: 812268 , 816705 , 829097 Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: I5084dd578682b2a8c128487c62de12e437e646f3 Reviewed-on: https://chromium-review.googlesource.com/981787 Commit-Queue: Wez <wez@chromium.org> Reviewed-by: John Budorick <jbudorick@chromium.org> Reviewed-by: Misha Efimov <mef@chromium.org> Cr-Commit-Position: refs/heads/master@{#548672} [modify] https://crrev.com/8fa7ecedfa0a5de4a27d93979d330b91e531b00c/components/cronet/cronet_global_state_stubs.cc [modify] https://crrev.com/8fa7ecedfa0a5de4a27d93979d330b91e531b00c/components/cronet/native/runnables_unittest.cc [modify] https://crrev.com/8fa7ecedfa0a5de4a27d93979d330b91e531b00c/components/cronet/native/test/buffer_test.cc [modify] https://crrev.com/8fa7ecedfa0a5de4a27d93979d330b91e531b00c/components/cronet/native/test/executors_test.cc [modify] https://crrev.com/8fa7ecedfa0a5de4a27d93979d330b91e531b00c/components/cronet/native/test/url_request_test.cc [modify] https://crrev.com/8fa7ecedfa0a5de4a27d93979d330b91e531b00c/components/cronet/native/url_request.cc [modify] https://crrev.com/8fa7ecedfa0a5de4a27d93979d330b91e531b00c/components/cronet/stale_host_resolver.cc [modify] https://crrev.com/8fa7ecedfa0a5de4a27d93979d330b91e531b00c/components/cronet/stale_host_resolver_unittest.cc [modify] https://crrev.com/8fa7ecedfa0a5de4a27d93979d330b91e531b00c/testing/buildbot/chromium.clang.json [modify] https://crrev.com/8fa7ecedfa0a5de4a27d93979d330b91e531b00c/testing/buildbot/chromium.fyi.json [modify] https://crrev.com/8fa7ecedfa0a5de4a27d93979d330b91e531b00c/testing/buildbot/chromium.linux.json [modify] https://crrev.com/8fa7ecedfa0a5de4a27d93979d330b91e531b00c/testing/buildbot/chromium.mac.json [modify] https://crrev.com/8fa7ecedfa0a5de4a27d93979d330b91e531b00c/testing/buildbot/chromium.memory.json [modify] https://crrev.com/8fa7ecedfa0a5de4a27d93979d330b91e531b00c/testing/buildbot/chromium.win.json [modify] https://crrev.com/8fa7ecedfa0a5de4a27d93979d330b91e531b00c/testing/buildbot/test_suites.pyl
,
Apr 6 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c9d03f9ef7d636af0719b8c0cadfe46856e28c70 commit c9d03f9ef7d636af0719b8c0cadfe46856e28c70 Author: Stephen McGruer <smcgruer@chromium.org> Date: Fri Apr 06 14:27:42 2018 Revert "Enable Cronet tests on desktop platforms." This reverts commit 8fa7ecedfa0a5de4a27d93979d330b91e531b00c. Reason for revert: HistogramManager.HistogramBucketFields crashes consistently on Win7 Tests (dbg): https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=cronet_unittests&tests=HistogramManager.HistogramBucketFields First failing run: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win7%20Tests%20%28dbg%29%281%29/68309 Original change's description: > Enable Cronet tests on desktop platforms. > > - Only initialize global state (e.g. AtExitManager) in non-Debug builds, so that our component- > build Debug bots can run cronet_tests. > - Fix memory leaks in a number of unit-tests. > - Disable some StaleHostResolver tests under ASAN, since they have leaks > that need resolving. > > Bug: 812268 , 816705 , 829097 > Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet > Change-Id: I5084dd578682b2a8c128487c62de12e437e646f3 > Reviewed-on: https://chromium-review.googlesource.com/981787 > Commit-Queue: Wez <wez@chromium.org> > Reviewed-by: John Budorick <jbudorick@chromium.org> > Reviewed-by: Misha Efimov <mef@chromium.org> > Cr-Commit-Position: refs/heads/master@{#548672} TBR=wez@chromium.org,mef@chromium.org,jbudorick@chromium.org Change-Id: Ic30729594ae2b1264702c7bb77c971d55e5e9ae9 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 812268 , 816705 , 829097 Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet Reviewed-on: https://chromium-review.googlesource.com/999334 Reviewed-by: Stephen McGruer <smcgruer@chromium.org> Commit-Queue: Stephen McGruer <smcgruer@chromium.org> Cr-Commit-Position: refs/heads/master@{#548771} [modify] https://crrev.com/c9d03f9ef7d636af0719b8c0cadfe46856e28c70/components/cronet/cronet_global_state_stubs.cc [modify] https://crrev.com/c9d03f9ef7d636af0719b8c0cadfe46856e28c70/components/cronet/native/runnables_unittest.cc [modify] https://crrev.com/c9d03f9ef7d636af0719b8c0cadfe46856e28c70/components/cronet/native/test/buffer_test.cc [modify] https://crrev.com/c9d03f9ef7d636af0719b8c0cadfe46856e28c70/components/cronet/native/test/executors_test.cc [modify] https://crrev.com/c9d03f9ef7d636af0719b8c0cadfe46856e28c70/components/cronet/native/test/url_request_test.cc [modify] https://crrev.com/c9d03f9ef7d636af0719b8c0cadfe46856e28c70/components/cronet/native/url_request.cc [modify] https://crrev.com/c9d03f9ef7d636af0719b8c0cadfe46856e28c70/components/cronet/stale_host_resolver.cc [modify] https://crrev.com/c9d03f9ef7d636af0719b8c0cadfe46856e28c70/components/cronet/stale_host_resolver_unittest.cc [modify] https://crrev.com/c9d03f9ef7d636af0719b8c0cadfe46856e28c70/testing/buildbot/chromium.clang.json [modify] https://crrev.com/c9d03f9ef7d636af0719b8c0cadfe46856e28c70/testing/buildbot/chromium.fyi.json [modify] https://crrev.com/c9d03f9ef7d636af0719b8c0cadfe46856e28c70/testing/buildbot/chromium.linux.json [modify] https://crrev.com/c9d03f9ef7d636af0719b8c0cadfe46856e28c70/testing/buildbot/chromium.mac.json [modify] https://crrev.com/c9d03f9ef7d636af0719b8c0cadfe46856e28c70/testing/buildbot/chromium.memory.json [modify] https://crrev.com/c9d03f9ef7d636af0719b8c0cadfe46856e28c70/testing/buildbot/chromium.win.json [modify] https://crrev.com/c9d03f9ef7d636af0719b8c0cadfe46856e28c70/testing/buildbot/test_suites.pyl
,
Apr 6 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/37176bc20f476b852a6139d1fa3d1fda76791709 commit 37176bc20f476b852a6139d1fa3d1fda76791709 Author: Wez <wez@chromium.org> Date: Fri Apr 06 20:07:44 2018 Reland "Enable Cronet tests on desktop platforms." This is a reland of 8fa7ecedfa0a5de4a27d93979d330b91e531b00c, which revealed several issues: - Histogram recording did not cope correctly with an empty metrics proto. - Several call-sites triggered MSan failures (e.g. due to read of uninitialized data). Original change's description: > Enable Cronet tests on desktop platforms. > > - Only initialize global state (e.g. AtExitManager) in non-Debug builds, so that our component- > build Debug bots can run cronet_tests. > - Fix memory leaks in a number of unit-tests. > - Disable some StaleHostResolver tests under ASAN, since they have leaks > that need resolving. > > Bug: 812268 , 816705 , 829097 > Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet > Change-Id: I5084dd578682b2a8c128487c62de12e437e646f3 > Reviewed-on: https://chromium-review.googlesource.com/981787 > Commit-Queue: Wez <wez@chromium.org> > Reviewed-by: John Budorick <jbudorick@chromium.org> > Reviewed-by: Misha Efimov <mef@chromium.org> > Cr-Commit-Position: refs/heads/master@{#548672} Bug: 812268 , 816705 , 829097 , 786559 Change-Id: Ia4027c994d475c48673c8e961f5d96255e703a15 Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.linux:linux_chromium_msan_rel_ng;master.tryserver.chromium.linux:linux_chromium_asan_rel_ng Reviewed-on: https://chromium-review.googlesource.com/999932 Reviewed-by: Wez <wez@chromium.org> Reviewed-by: Misha Efimov <mef@chromium.org> Reviewed-by: John Budorick <jbudorick@chromium.org> Commit-Queue: Wez <wez@chromium.org> Cr-Commit-Position: refs/heads/master@{#548914} [modify] https://crrev.com/37176bc20f476b852a6139d1fa3d1fda76791709/components/cronet/cronet_global_state_stubs.cc [modify] https://crrev.com/37176bc20f476b852a6139d1fa3d1fda76791709/components/cronet/native/runnables_unittest.cc [modify] https://crrev.com/37176bc20f476b852a6139d1fa3d1fda76791709/components/cronet/native/test/buffer_test.cc [modify] https://crrev.com/37176bc20f476b852a6139d1fa3d1fda76791709/components/cronet/native/test/executors_test.cc [modify] https://crrev.com/37176bc20f476b852a6139d1fa3d1fda76791709/components/cronet/native/test/url_request_test.cc [modify] https://crrev.com/37176bc20f476b852a6139d1fa3d1fda76791709/components/cronet/native/url_request.cc [modify] https://crrev.com/37176bc20f476b852a6139d1fa3d1fda76791709/components/cronet/stale_host_resolver.cc [modify] https://crrev.com/37176bc20f476b852a6139d1fa3d1fda76791709/components/cronet/stale_host_resolver_unittest.cc [modify] https://crrev.com/37176bc20f476b852a6139d1fa3d1fda76791709/testing/buildbot/chromium.clang.json [modify] https://crrev.com/37176bc20f476b852a6139d1fa3d1fda76791709/testing/buildbot/chromium.fyi.json [modify] https://crrev.com/37176bc20f476b852a6139d1fa3d1fda76791709/testing/buildbot/chromium.linux.json [modify] https://crrev.com/37176bc20f476b852a6139d1fa3d1fda76791709/testing/buildbot/chromium.mac.json [modify] https://crrev.com/37176bc20f476b852a6139d1fa3d1fda76791709/testing/buildbot/chromium.memory.json [modify] https://crrev.com/37176bc20f476b852a6139d1fa3d1fda76791709/testing/buildbot/chromium.win.json [modify] https://crrev.com/37176bc20f476b852a6139d1fa3d1fda76791709/testing/buildbot/test_suites.pyl
,
Apr 16 2018
IIRC we had reached agreement that r433525 was incorrect since one of the downsides include "causes many ODR violations and hence has undefined behavior". IIRC we had discussed this in some detail somewhere (irc? chat?) and I had filed bug 744567 for an alternate approach that allows you to retain the same test coverage without these downsides.
,
Apr 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5fe14697a2fc922c26818612a3e3f6ba22daaa99 commit 5fe14697a2fc922c26818612a3e3f6ba22daaa99 Author: Wez <wez@chromium.org> Date: Wed Apr 18 19:45:18 2018 [cronet] Apply AtExitManager workaround to Component builds. We work-around shared global state in component builds of the Cronet native library by avoiding creating the AtExitManager in those builds. The condition for that is fixed here, from being based on Debug build to depending directly on whether this is a component build. The tests are also enabled on Fuchsia/Debug, where they now pass. Bug: 833401 , 816705 Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: I1401c0dcaa8e893c6cd4bcbf75f8d4d30ccd0116 Reviewed-on: https://chromium-review.googlesource.com/1014642 Commit-Queue: Wez <wez@chromium.org> Reviewed-by: Dirk Pranke <dpranke@chromium.org> Reviewed-by: Reid Kleckner <rnk@chromium.org> Reviewed-by: Misha Efimov <mef@chromium.org> Cr-Commit-Position: refs/heads/master@{#551788} [modify] https://crrev.com/5fe14697a2fc922c26818612a3e3f6ba22daaa99/components/cronet/cronet_global_state_stubs.cc [modify] https://crrev.com/5fe14697a2fc922c26818612a3e3f6ba22daaa99/testing/buildbot/chromium.fyi.json [modify] https://crrev.com/5fe14697a2fc922c26818612a3e3f6ba22daaa99/testing/buildbot/test_suite_exceptions.pyl
,
Apr 18 2018
Marking this as Fixed, since cronet_tests now pass in component builds. Issue 744567 tracks follow-up work to properly address cronet_tests under component builds. |
||
►
Sign in to add a comment |
||
Comment 1 by pauljensen@chromium.org
, Feb 27 2018