|
|
Chromium Code Reviews|
Created:
8 years, 5 months ago by jam Modified:
8 years, 5 months ago Reviewers:
mmenke CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam, jochen+watch-content_chromium.org Base URL:
svn://chrome-svn/chrome/trunk/src/ Visibility:
Public. |
DescriptionMake the empty browser_test work again after incremental linking broke it.
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=145720
Patch Set 1 #
Total comments: 1
Patch Set 2 : sync #
Messages
Total messages: 11 (0 generated)
We'll still be trying to run it, and failing, in interactive_ui_tests. Do we care? http://codereview.chromium.org/10692048/diff/1/content/test/test_launcher.cc File content/test/test_launcher.cc (right): http://codereview.chromium.org/10692048/diff/1/content/test/test_launcher.cc#... content/test/test_launcher.cc:56: const char kEmptyTestName[] = "InProcessBrowserTest.Empty"; Kinda weird to still have this here, though getting rid of it again would probably be a bit of a pain.
Have we had a problem with interactive_ui_tests in the past? On Fri, Jun 29, 2012 at 1:11 PM, <mmenke@chromium.org> wrote: > We'll still be trying to run it, and failing, in interactive_ui_tests. Do > we > care? > > > http://codereview.chromium.**org/10692048/diff/1/content/** > test/test_launcher.cc<http://codereview.chromium.org/10692048/diff/1/content/test/test_launcher.cc> > File content/test/test_launcher.cc (right): > > http://codereview.chromium.**org/10692048/diff/1/content/** > test/test_launcher.cc#**newcode56<http://codereview.chromium.org/10692048/diff/1/content/test/test_launcher.cc#newcode56> > content/test/test_launcher.cc:**56: const char kEmptyTestName[] = > "InProcessBrowserTest.Empty"; > Kinda weird to still have this here, though getting rid of it again > would probably be a bit of a pain. > > http://codereview.chromium.**org/10692048/<http://codereview.chromium.org/106... >
Not that I'm aware of. Single threaded warmup time seems like it's much less of an issue than multi-threaded warmup time, at least from the logs I've looked at. Disclaimer: Having never written an interactive_ui_test, I don't exactly look at their results often. On 2012/06/29 20:15:25, John Abd-El-Malek wrote: > Have we had a problem with interactive_ui_tests in the past? > > On Fri, Jun 29, 2012 at 1:11 PM, <mailto:mmenke@chromium.org> wrote: > > > We'll still be trying to run it, and failing, in interactive_ui_tests. Do > > we > > care? > > > > > > http://codereview.chromium.**org/10692048/diff/1/content/** > > > test/test_launcher.cc<http://codereview.chromium.org/10692048/diff/1/content/test/test_launcher.cc> > > File content/test/test_launcher.cc (right): > > > > http://codereview.chromium.**org/10692048/diff/1/content/** > > > test/test_launcher.cc#**newcode56<http://codereview.chromium.org/10692048/diff/1/content/test/test_launcher.cc#newcode56> > > content/test/test_launcher.cc:**56: const char kEmptyTestName[] = > > "InProcessBrowserTest.Empty"; > > Kinda weird to still have this here, though getting rid of it again > > would probably be a bit of a pain. > > > > > http://codereview.chromium.**org/10692048/%3Chttp://codereview.chromium.org/1...> > >
Not sure if you're expecting a signoff on this from me or not. The change LGTM. Looking at the interactive_ui warmup times, it's not really clear if they matter or not. They're currently not being run by the buildbots, only the trybots, which usually run release builds. Startup time only really seems to be an issue with debug builds.
On Fri, Jun 29, 2012 at 1:57 PM, <mmenke@chromium.org> wrote: > Not sure if you're expecting a signoff on this from me or not. The change > LGTM. > I thought you might want to patch it locally to test. Want to do that? > > Looking at the interactive_ui warmup times, it's not really clear if they > matter > or not. They're currently not being run by the buildbots, only the > trybots, > which usually run release builds. Startup time only really seems to be an > issue > with debug builds. > They are run on the buildbots (both release+debug). > > http://codereview.chromium.**org/10692048/<http://codereview.chromium.org/106... >
On 2012/06/29 21:06:00, John Abd-El-Malek wrote: > On Fri, Jun 29, 2012 at 1:57 PM, <mailto:mmenke@chromium.org> wrote: > > > Not sure if you're expecting a signoff on this from me or not. The change > > LGTM. > > > > I thought you might want to patch it locally to test. Want to do that? Sure, will do that later tonight. > > > > Looking at the interactive_ui warmup times, it's not really clear if they > > matter > > or not. They're currently not being run by the buildbots, only the > > trybots, > > which usually run release builds. Startup time only really seems to be an > > issue > > with debug builds. > > > > They are run on the buildbots (both release+debug). Doesn't look like they're currently being run. Looking at the waterfall, don't see them being run by any bots. Do see them being run on try jobs.
On 2012/06/29 21:10:47, Matt Menke wrote: > On 2012/06/29 21:06:00, John Abd-El-Malek wrote: > > On Fri, Jun 29, 2012 at 1:57 PM, <mailto:mmenke@chromium.org> wrote: > > > > > Not sure if you're expecting a signoff on this from me or not. The change > > > LGTM. > > > > > > > I thought you might want to patch it locally to test. Want to do that? > > Sure, will do that later tonight. I've confirmed this works.
On Fri, Jun 29, 2012 at 2:10 PM, <mmenke@chromium.org> wrote: > On 2012/06/29 21:06:00, John Abd-El-Malek wrote: > > On Fri, Jun 29, 2012 at 1:57 PM, <mailto:mmenke@chromium.org> wrote: >> > > > Not sure if you're expecting a signoff on this from me or not. The >> change >> > LGTM. >> > >> > > I thought you might want to patch it locally to test. Want to do that? >> > > Sure, will do that later tonight. > > > > >> > Looking at the interactive_ui warmup times, it's not really clear if >> they >> > matter >> > or not. They're currently not being run by the buildbots, only the >> > trybots, >> > which usually run release builds. Startup time only really seems to be >> an >> > issue >> > with debug builds. >> > >> > > They are run on the buildbots (both release+debug). >> > > Doesn't look like they're currently being run. Looking at the waterfall, > don't > see them being run by any bots. Do see them being run on try jobs. > wow, sorry you're right. something must have changed recently. they're running on debug windows here: http://build.chromium.org/p/chromium/builders/Interactive%20Tests%20%28dbg%29 but they should be running on mac/linux as well. i'll ping the infrastructure team > > http://codereview.chromium.**org/10692048/<http://codereview.chromium.org/106... >
On 2012/06/29 21:55:28, John Abd-El-Malek wrote: > On Fri, Jun 29, 2012 at 2:10 PM, <mailto:mmenke@chromium.org> wrote: > > > On 2012/06/29 21:06:00, John Abd-El-Malek wrote: > > > > On Fri, Jun 29, 2012 at 1:57 PM, <mailto:mmenke@chromium.org> wrote: > >> > > > > > Not sure if you're expecting a signoff on this from me or not. The > >> change > >> > LGTM. > >> > > >> > > > > I thought you might want to patch it locally to test. Want to do that? > >> > > > > Sure, will do that later tonight. > > > > > > > > >> > Looking at the interactive_ui warmup times, it's not really clear if > >> they > >> > matter > >> > or not. They're currently not being run by the buildbots, only the > >> > trybots, > >> > which usually run release builds. Startup time only really seems to be > >> an > >> > issue > >> > with debug builds. > >> > > >> > > > > They are run on the buildbots (both release+debug). > >> > > > > Doesn't look like they're currently being run. Looking at the waterfall, > > don't > > see them being run by any bots. Do see them being run on try jobs. > > > > wow, sorry you're right. something must have changed recently. > they're running on debug windows here: > http://build.chromium.org/p/chromium/builders/Interactive%2520Tests%2520%2528... > > but they should be running on mac/linux as well. i'll ping the > infrastructure team So now that the right bots have been pointed out to me, looks like the issue also exists on the Windows interactive_ui bot. [==========] Running 1 test from 1 test case. [----------] Global test environment set-up. [----------] 1 test from MouseLeaveTest, where TypeParam = [ RUN ] MouseLeaveTest.TestOnMouseOut [1016:2120:0629/140625:2968539218:ERROR:external_registry_extension_loader_win.cc(87)] Missing value path for key Software\Google\Chrome\Extensions\jfmjfhklogoienhpfnppmbcbjfjnkonk. [ OK ] MouseLeaveTest.TestOnMouseOut (25515 ms) Typical test takes 6 to 14 seconds. Suspect that since only one is running at a time, it would take a much slower test to knock it up to 45 seconds, but it's still within the realm of possibility.
On Fri, Jun 29, 2012 at 3:17 PM, <mmenke@chromium.org> wrote: > On 2012/06/29 21:55:28, John Abd-El-Malek wrote: > > On Fri, Jun 29, 2012 at 2:10 PM, <mailto:mmenke@chromium.org> wrote: >> > > > On 2012/06/29 21:06:00, John Abd-El-Malek wrote: >> > >> > On Fri, Jun 29, 2012 at 1:57 PM, <mailto:mmenke@chromium.org> wrote: >> >> >> > >> > > Not sure if you're expecting a signoff on this from me or not. The >> >> change >> >> > LGTM. >> >> > >> >> >> > >> > I thought you might want to patch it locally to test. Want to do that? >> >> >> > >> > Sure, will do that later tonight. >> > >> > >> > > >> >> > Looking at the interactive_ui warmup times, it's not really clear if >> >> they >> >> > matter >> >> > or not. They're currently not being run by the buildbots, only the >> >> > trybots, >> >> > which usually run release builds. Startup time only really seems to >> be >> >> an >> >> > issue >> >> > with debug builds. >> >> > >> >> >> > >> > They are run on the buildbots (both release+debug). >> >> >> > >> > Doesn't look like they're currently being run. Looking at the >> waterfall, >> > don't >> > see them being run by any bots. Do see them being run on try jobs. >> > >> > > wow, sorry you're right. something must have changed recently. >> they're running on debug windows here: >> > > http://build.chromium.org/p/**chromium/builders/Interactive%** > 2520Tests%2520%2528dbg%2529<http://build.chromium.org/p/chromium/builders/Interactive%2520Tests%2520%2528dbg%2529> > > > but they should be running on mac/linux as well. i'll ping the >> infrastructure team >> > > So now that the right bots have been pointed out to me, looks like the > issue > also exists on the Windows interactive_ui bot. > > [==========] Running 1 test from 1 test case. > [----------] Global test environment set-up. > [----------] 1 test from MouseLeaveTest, where TypeParam = > [ RUN ] MouseLeaveTest.TestOnMouseOut > [1016:2120:0629/140625:**2968539218:ERROR:external_** > registry_extension_loader_win.**cc(87)] > Missing value path for key > Software\Google\Chrome\**Extensions\**jfmjfhklogoienhpfnppmbcbjfjnko**nk. > [ OK ] MouseLeaveTest.TestOnMouseOut (25515 ms) > > Typical test takes 6 to 14 seconds. Suspect that since only one is > running at a > time, it would take a much slower test to knock it up to 45 seconds, but > it's > still within the realm of possibility. > ok, how about we just do your change for browser_tests for now. if we find issues with interactive_ui_tests, we can try to generalize this. > > http://codereview.chromium.**org/10692048/<http://codereview.chromium.org/106... >
On 2012/06/29 22:36:42, John Abd-El-Malek wrote: > ok, how about we just do your change for browser_tests for now. if we find > issues with interactive_ui_tests, we can try to generalize this. SGTM (Though, technically, the interactive_ui_tests have been trying to run the empty test unsuccessfully for the last month, and my change deliberately didn't change that behavior). |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
