base_unittests and unit_tests failures in win-asan official builder. |
||||||
Issue descriptionThis builder produces an official/Release build with dcheck_is_configurable=true. Death-test failures typically mean the test's expectations do not correctly match the [D]CHECKs in the code, e.g. they use EXPECT_DCHECK_DEATH() but the code under test uses CHECK. base_unittests failing: RunLoopDeathTest.MustRegisterBeforeInstantiating ObserverListTest.NonReentrantObserverList SampleVectorTest.AddSubtractBucketNotMatchDeath SampleVectorTest.BucketIndexDeath TestMockTimeTaskRunnerTest.DefaultUnbound unit_tests failing: ExtensionUpdaterTest.TestExtensionUpdateCheckRequestsPending ExtensionUpdaterTest.TestInstallSource ExtensionUpdaterTest.TestUpdateUrlData ExtensionUpdaterTest.TestExtensionUpdateCheckRequests ExtensionUpdaterTest.TestInstallLocation
,
Apr 13 2018
RunLoopDeathTest.MustRegisterBeforeInstantiating fails because it EXPECT_DCHECK_DEATH()s but the code-under-test hits a CHECK() before reaching the intended DCHECK(). +gab@, who added this test in https://chromium-review.googlesource.com/594352
,
Apr 14 2018
ExtensionUpdaterTest.* tests are all failing because: 1. |prodchannel| is populated from channel_info::GetChannelInfo(), which returns " DCheck" in these builds, instead of "". 2. The tests verify that the UpdateQueryParams() are correctly copied into the query URL, but something in that path seems to strip the leading space. +rdevlin.cronin@ - Devlin, how is the |prodchannel| field used..? Does returning "<channel> DCheck" here actually pose a problem in production?
,
Apr 14 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/39ee610f7bfa84da3bcdb33c31d126cfc4081905 commit 39ee610f7bfa84da3bcdb33c31d126cfc4081905 Author: Wez <wez@chromium.org> Date: Sat Apr 14 21:33:59 2018 Fix RunLoopDeathTest to expect CHECK rather than DCHECK failure. Bug: 832728 , 703346 Change-Id: I4f400986db39b957095a30dc7770f6e6e541a8ae Reviewed-on: https://chromium-review.googlesource.com/1012568 Commit-Queue: Wez <wez@chromium.org> Reviewed-by: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#550908} [modify] https://crrev.com/39ee610f7bfa84da3bcdb33c31d126cfc4081905/base/run_loop_unittest.cc
,
Apr 17 2018
@3 Sorry, I don't quite follow. I don't see any field called prodchannel in the extension updater, or any assertions for dcheck vs not in ExtensionUpdaterTest. I also can't find any calls to GetChannelInfo() in the updater. Can you include some code pointers to the places you're referencing?
,
Apr 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/39ee610f7bfa84da3bcdb33c31d126cfc4081905 commit 39ee610f7bfa84da3bcdb33c31d126cfc4081905 Author: Wez <wez@chromium.org> Date: Sat Apr 14 21:33:59 2018 Fix RunLoopDeathTest to expect CHECK rather than DCHECK failure. Bug: 832728 , 703346 Change-Id: I4f400986db39b957095a30dc7770f6e6e541a8ae Reviewed-on: https://chromium-review.googlesource.com/1012568 Commit-Queue: Wez <wez@chromium.org> Reviewed-by: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#550908} [modify] https://crrev.com/39ee610f7bfa84da3bcdb33c31d126cfc4081905/base/run_loop_unittest.cc
,
Apr 17 2018
Re #5: Sorry, I'm referring to the update query URL verification at https://cs.chromium.org/chromium/src/chrome/browser/extensions/updater/extension_updater_unittest.cc?l=623. The function is basically checking that the general update query parameters are all present and correct in extension updater query URLs, in addition to the extension-specific bits. For the "expected" query params, "prodchannel" is a field inserted into the update query URL retrieved from UpdateQueryParams::Get() (see https://cs.chromium.org/chromium/src/components/update_client/update_query_params.cc?l=75) by the UpdateQueryParamsDelegate (see Chrome's impl https://cs.chromium.org/chromium/src/chrome/browser/update_client/chrome_update_query_params_delegate.cc?l=32 which adds "prodchannel" based on GetChannelInfo()). The failing tests are calling the same UpdateQueryParams::Get() function to get the query parameters (see https://cs.chromium.org/chromium/src/chrome/browser/extensions/updater/extension_updater_unittest.cc?l=2013) to pass when creating the ManifestFetchData helper. Internally that helper calls GURL::ReplaceComponents() (see https://cs.chromium.org/chromium/src/extensions/browser/updater/manifest_fetch_data.cc?sq=package:chromium&l=95), which I suspect is where the space is getting stripped, from " DCheck" to "DCheck". This may be as simple as changing the " DCheck" channel suffix to e.g. "-DCheck", but I'm surprised that ReplaceComponents() seems to be stripping characters from elements.
,
Apr 17 2018
+component updater experts
,
Apr 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f0acd4fcf1939a152c2787f625644a8e57947d28 commit f0acd4fcf1939a152c2787f625644a8e57947d28 Author: Wez <wez@chromium.org> Date: Tue Apr 17 21:39:44 2018 Fix SampleVector tests to expect CHECK rather than DCHECK deaths. Bug: 832728 Change-Id: I9a2d2d330aa4d250c925b7a614780acbf40dce3f Reviewed-on: https://chromium-review.googlesource.com/1012732 Reviewed-by: Alexei Svitkine <asvitkine@chromium.org> Commit-Queue: Wez <wez@chromium.org> Cr-Commit-Position: refs/heads/master@{#551483} [modify] https://crrev.com/f0acd4fcf1939a152c2787f625644a8e57947d28/base/metrics/sample_vector_unittest.cc
,
Apr 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a10be3237416a9a944d4cc024e8b4c6bbaad5d33 commit a10be3237416a9a944d4cc024e8b4c6bbaad5d33 Author: Findit <findit-for-me@appspot.gserviceaccount.com> Date: Tue Apr 17 22:58:32 2018 Revert "Fix SampleVector tests to expect CHECK rather than DCHECK deaths." This reverts commit f0acd4fcf1939a152c2787f625644a8e57947d28. Reason for revert: Findit (https://goo.gl/kROfz5) identified CL at revision 551483 as the culprit for failures in the build cycles as shown on: https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2YwYWNkNGZjZjE5MzlhMTUyYzI3ODdmNjI1NjQ0YThlNTc5NDdkMjgM Sample Failed Build: https://ci.chromium.org/buildbot/chromium.chromiumos/linux-chromeos-rel/7001 Sample Failed Step: base_unittests Original change's description: > Fix SampleVector tests to expect CHECK rather than DCHECK deaths. > > Bug: 832728 > Change-Id: I9a2d2d330aa4d250c925b7a614780acbf40dce3f > Reviewed-on: https://chromium-review.googlesource.com/1012732 > Reviewed-by: Alexei Svitkine <asvitkine@chromium.org> > Commit-Queue: Wez <wez@chromium.org> > Cr-Commit-Position: refs/heads/master@{#551483} Change-Id: Ie2ad7fb9ebd4c214a7dd2cebe0763b2f41793f01 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 832728 Reviewed-on: https://chromium-review.googlesource.com/1015194 Cr-Commit-Position: refs/heads/master@{#551512} [modify] https://crrev.com/a10be3237416a9a944d4cc024e8b4c6bbaad5d33/base/metrics/sample_vector_unittest.cc
,
Apr 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8aefefd90c62830a6b17606011c2c1fea542f2b3 commit 8aefefd90c62830a6b17606011c2c1fea542f2b3 Author: Wez <wez@chromium.org> Date: Wed Apr 18 19:17:30 2018 Reland "Fix SampleVector tests to expect CHECK rather than DCHECK deaths." This is a reland of f0acd4fcf1939a152c2787f625644a8e57947d28, which mistakenly replaced some EXPECT_DCHECK_DEATH() checks with EXPECT_DEATH() even though the code under test really was DCHECKing. Original change's description: > Fix SampleVector tests to expect CHECK rather than DCHECK deaths. > > Bug: 832728 > Change-Id: I9a2d2d330aa4d250c925b7a614780acbf40dce3f > Reviewed-on: https://chromium-review.googlesource.com/1012732 > Reviewed-by: Alexei Svitkine <asvitkine@chromium.org> > Commit-Queue: Wez <wez@chromium.org> > Cr-Commit-Position: refs/heads/master@{#551483} Bug: 832728 Change-Id: I046d3ad35a9b9b7e6d3f214029dfcfaeba81243d Reviewed-on: https://chromium-review.googlesource.com/1016861 Commit-Queue: Wez <wez@chromium.org> Reviewed-by: Alexei Svitkine <asvitkine@chromium.org> Cr-Commit-Position: refs/heads/master@{#551776} [modify] https://crrev.com/8aefefd90c62830a6b17606011c2c1fea542f2b3/base/metrics/sample_vector_unittest.cc
,
Apr 24 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c536c3c91a02d679f532a46db12aa5e91178a174 commit c536c3c91a02d679f532a46db12aa5e91178a174 Author: Wez <wez@chromium.org> Date: Tue Apr 24 01:22:25 2018 Don't use whitespace in ChannelName() under Windows Official DCHECK. Using whitespace breaks ExtensionUpdater query parameter generation. Bug: 832728 Change-Id: I191d175d623803d4655d22caf422bf7127bdc57c Reviewed-on: https://chromium-review.googlesource.com/1016138 Reviewed-by: Jochen Eisinger <jochen@chromium.org> Commit-Queue: Wez <wez@chromium.org> Cr-Commit-Position: refs/heads/master@{#552942} [modify] https://crrev.com/c536c3c91a02d679f532a46db12aa5e91178a174/chrome/common/channel_info_win.cc
,
Apr 24 2018
Just to update the latest behavior of this issue, still below failures is observed for win-asan official.desktop builder. Failures: ----------- TestMockTimeTaskRunnerTest.DefaultUnbound ObserverListTest.NonReentrantObserverList Link to builder: --------------- https://uberchromegw.corp.google.com/i/official.desktop/builders/win-asan/builds/945 Link to log: ------------ https://logs.chromium.org/v/?s=chrome%2Fbb%2Fofficial.desktop%2Fwin-asan%2F945%2F%2B%2Frecipes%2Fsteps%2Fbase_unittests%2F0%2Fstdout Thanks!
,
Apr 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0c9c93cef24767d3bc2f069d727aee144f693c50 commit 0c9c93cef24767d3bc2f069d727aee144f693c50 Author: Wez <wez@chromium.org> Date: Thu Apr 26 16:23:24 2018 Tweak some death test expectations to pass dcheck_is_configurable. - Fixes TestMockTimeTaskRunnerTest to use EXPECT_DEATH(). - Switches ObserverListTest.NonReentrantObserverList to use the EXPECT_DCHECK_DEATH() macro rather than the ScopedLogAssertHandler, so that LOG_DCHECK will be set correctly for the to-die child process. Bug: 832728 Change-Id: I7f90bfd49491520c49505dc1500d3934aad81494 Reviewed-on: https://chromium-review.googlesource.com/1027990 Commit-Queue: Wez <wez@chromium.org> Reviewed-by: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#554046} [modify] https://crrev.com/0c9c93cef24767d3bc2f069d727aee144f693c50/base/observer_list_unittest.cc [modify] https://crrev.com/0c9c93cef24767d3bc2f069d727aee144f693c50/base/test/test_mock_time_task_runner_unittest.cc
,
Apr 27 2018
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by w...@chromium.org
, Apr 13 2018