Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2032613004:
Move remaining tests on win GN bots to main bots and update CQ (Closed)
|
Created:
4 years, 6 months ago by Dirk Pranke Modified:
4 years, 6 months ago CC:
chromium-reviews Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionMove remaining tests on win GN bots to main bots and update CQ
There were a few test steps running only on the Win8 Aura
bot, which was the original GN bot. This CL makes sure
they run on the main Windows x86 and x64 bots now that
they are also using GN.
This CL also takes win8_chromium_ng out of the CQ since
it's now redundant.
R=brucedawson@chromium.org, msw@chromium.org
BUG=605318
Committed: https://crrev.com/caa330a53f0d7972ae3fa276f0ade9f0e33e3037
Cr-Commit-Position: refs/heads/master@{#397982}
Patch Set 1 #
Total comments: 16
Messages
Total messages: 18 (4 generated)
Description was changed from ========== Move remaining "GN" tests on win bots to regular bots. There were a few test steps running only on the Win8 Aura bot, which was the original GN bot. This CL makes sure they run on the main Windows x86 and x64 bots now that they are also using GN. R=brucedawson@chromium.org, msw@chromium.org BUG=605318 ========== to ========== Move remaining tests on win GN bots to main bots and update CQ There were a few test steps running only on the Win8 Aura bot, which was the original GN bot. This CL makes sure they run on the main Windows x86 and x64 bots now that they are also using GN. This CL also takes win8_chromium_ng out of the CQ since it's now redundant. R=brucedawson@chromium.org, msw@chromium.org BUG=605318 ==========
Just some questions, mostly lg https://codereview.chromium.org/2032613004/diff/1/infra/config/cq.cfg File infra/config/cq.cfg (left): https://codereview.chromium.org/2032613004/diff/1/infra/config/cq.cfg#oldcode78 infra/config/cq.cfg:78: builders { name: "win8_chromium_ng" } Will you remove this bot in an infra repo CL? Should you remove more src repo references now? https://codereview.chromium.org/2032613004/diff/1/testing/buildbot/chromium.w... File testing/buildbot/chromium.win.json (left): https://codereview.chromium.org/2032613004/diff/1/testing/buildbot/chromium.w... testing/buildbot/chromium.win.json:441: "gn_all" Should we keep this in an additional_compile_targets list somewhere? https://codereview.chromium.org/2032613004/diff/1/testing/buildbot/chromium.w... testing/buildbot/chromium.win.json:2159: "ash_shell_with_content", Ditto? https://codereview.chromium.org/2032613004/diff/1/testing/buildbot/chromium.w... testing/buildbot/chromium.win.json:2162: "battor_agent", Ditto? (Maybe battor_agent_unittests is sufficient) https://codereview.chromium.org/2032613004/diff/1/testing/buildbot/chromium.w... testing/buildbot/chromium.win.json:2166: "chrome", Ditto? (Maybe browser/interactive_ui_tests is sufficient) https://codereview.chromium.org/2032613004/diff/1/testing/buildbot/chromium.w... testing/buildbot/chromium.win.json:2189: "mash:all", Ditto? https://codereview.chromium.org/2032613004/diff/1/testing/buildbot/chromium.w... testing/buildbot/chromium.win.json:2195: "remoting_all", Ditto?
https://codereview.chromium.org/2032613004/diff/1/infra/config/cq.cfg File infra/config/cq.cfg (left): https://codereview.chromium.org/2032613004/diff/1/infra/config/cq.cfg#oldcode78 infra/config/cq.cfg:78: builders { name: "win8_chromium_ng" } On 2016/06/02 19:24:50, msw wrote: > Will you remove this bot in an infra repo CL? Yes. > Should you remove more src repo references now? I'll probably do that in follow-up CLs once the bot is actually gone. https://codereview.chromium.org/2032613004/diff/1/testing/buildbot/chromium.w... File testing/buildbot/chromium.win.json (left): https://codereview.chromium.org/2032613004/diff/1/testing/buildbot/chromium.w... testing/buildbot/chromium.win.json:441: "gn_all" On 2016/06/02 19:24:51, msw wrote: > Should we keep this in an additional_compile_targets list somewhere? Possibly, but we're out of SLO in the win CQ at the moment and trying to free up capacity, so I'd like to leave it out for now and then add it back in when we have headroom again. https://codereview.chromium.org/2032613004/diff/1/testing/buildbot/chromium.w... testing/buildbot/chromium.win.json:2159: "ash_shell_with_content", On 2016/06/02 19:24:50, msw wrote: > Ditto? I can add this if it's not pulled in otherwise (will check). https://codereview.chromium.org/2032613004/diff/1/testing/buildbot/chromium.w... testing/buildbot/chromium.win.json:2162: "battor_agent", On 2016/06/02 19:24:50, msw wrote: > Ditto? (Maybe battor_agent_unittests is sufficient) Same answer. https://codereview.chromium.org/2032613004/diff/1/testing/buildbot/chromium.w... testing/buildbot/chromium.win.json:2166: "chrome", On 2016/06/02 19:24:51, msw wrote: > Ditto? (Maybe browser/interactive_ui_tests is sufficient) Yup, browser_tests pulls in chrome. https://codereview.chromium.org/2032613004/diff/1/testing/buildbot/chromium.w... testing/buildbot/chromium.win.json:2189: "mash:all", On 2016/06/02 19:24:51, msw wrote: > Ditto? Will add. https://codereview.chromium.org/2032613004/diff/1/testing/buildbot/chromium.w... testing/buildbot/chromium.win.json:2195: "remoting_all", On 2016/06/02 19:24:50, msw wrote: > Ditto? Will add.
https://codereview.chromium.org/2032613004/diff/1/infra/config/cq.cfg File infra/config/cq.cfg (left): https://codereview.chromium.org/2032613004/diff/1/infra/config/cq.cfg#oldcode78 infra/config/cq.cfg:78: builders { name: "win8_chromium_ng" } On 2016/06/02 19:28:41, Dirk Pranke wrote: > On 2016/06/02 19:24:50, msw wrote: > > Will you remove this bot in an infra repo CL? > > Yes. > > > Should you remove more src repo references now? > > I'll probably do that in follow-up CLs once the bot is actually gone. Acknowledged. https://codereview.chromium.org/2032613004/diff/1/testing/buildbot/chromium.w... File testing/buildbot/chromium.win.json (left): https://codereview.chromium.org/2032613004/diff/1/testing/buildbot/chromium.w... testing/buildbot/chromium.win.json:441: "gn_all" On 2016/06/02 19:28:41, Dirk Pranke wrote: > On 2016/06/02 19:24:51, msw wrote: > > Should we keep this in an additional_compile_targets list somewhere? > > Possibly, but we're out of SLO in the win CQ at the moment and trying to > free up capacity, so I'd like to leave it out for now and then add it back > in when we have headroom again. Hmm, this is a little worrisome, some teams might be relying on this to ensure they don't have build regressions, right?
On 2016/06/02 20:31:31, msw wrote: > https://codereview.chromium.org/2032613004/diff/1/infra/config/cq.cfg > File infra/config/cq.cfg (left): > > https://codereview.chromium.org/2032613004/diff/1/infra/config/cq.cfg#oldcode78 > infra/config/cq.cfg:78: builders { name: "win8_chromium_ng" } > On 2016/06/02 19:28:41, Dirk Pranke wrote: > > On 2016/06/02 19:24:50, msw wrote: > > > Will you remove this bot in an infra repo CL? > > > > Yes. > > > > > Should you remove more src repo references now? > > > > I'll probably do that in follow-up CLs once the bot is actually gone. > > Acknowledged. > > https://codereview.chromium.org/2032613004/diff/1/testing/buildbot/chromium.w... > File testing/buildbot/chromium.win.json (left): > > https://codereview.chromium.org/2032613004/diff/1/testing/buildbot/chromium.w... > testing/buildbot/chromium.win.json:441: "gn_all" > On 2016/06/02 19:28:41, Dirk Pranke wrote: > > On 2016/06/02 19:24:51, msw wrote: > > > Should we keep this in an additional_compile_targets list somewhere? > > > > Possibly, but we're out of SLO in the win CQ at the moment and trying to > > free up capacity, so I'd like to leave it out for now and then add it back > > in when we have headroom again. > > Hmm, this is a little worrisome, some teams might be relying on this to ensure > they don't have build regressions, right? This is a general problem we've had in the CQ for a long time; the Mac and Win ports don't build "all" as part of the CQ, only on some continuous builders. This CL doesn't really change that. I would like to see us building 'all' everywhere in the CQ, but jam@ has always preferred shorter cycle times, so you've got to convince him first.
Can we get a gn builder or two building "all" on the waterfall instead of the CQ? There have been a few breaks (currently one in v8) that happen when doing gn builds of "all".
On 2016/06/02 20:41:03, Dirk Pranke wrote: > https://codereview.chromium.org/2032613004/diff/1/testing/buildbot/chromium.w... > > File testing/buildbot/chromium.win.json (left): > > > > > https://codereview.chromium.org/2032613004/diff/1/testing/buildbot/chromium.w... > > testing/buildbot/chromium.win.json:441: "gn_all" > > On 2016/06/02 19:28:41, Dirk Pranke wrote: > > > On 2016/06/02 19:24:51, msw wrote: > > > > Should we keep this in an additional_compile_targets list somewhere? > > > > > > Possibly, but we're out of SLO in the win CQ at the moment and trying to > > > free up capacity, so I'd like to leave it out for now and then add it back > > > in when we have headroom again. > > > > Hmm, this is a little worrisome, some teams might be relying on this to ensure > > they don't have build regressions, right? > > This is a general problem we've had in the CQ for a long time; the Mac and Win > ports don't build "all" as part of the CQ, only on some continuous builders. > > This CL doesn't really change that. I would like to see us building 'all' > everywhere > in the CQ, but jam@ has always preferred shorter cycle times, so you've got to > convince him first. okay; lgtm!
On 2016/06/02 20:46:49, brucedawson wrote: > Can we get a gn builder or two building "all" on the waterfall instead of the > CQ? There have been a few breaks (currently one in v8) that happen when doing gn > builds of "all". Approve the CL I sent you and we will (we build all on the "Win" and "Win x64" bots on the chromium waterfall, for starters).
> Approve the CL I sent you and we will (we build all on the "Win" and "Win x64" > bots on the chromium waterfall, for starters). Gotcha. I'm working with the v8 team on unblocking that CL (https://codereview.chromium.org/2030823002/)
lgtm
The CQ bit was checked by dpranke@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2032613004/1
Message was sent while issue was closed.
Description was changed from ========== Move remaining tests on win GN bots to main bots and update CQ There were a few test steps running only on the Win8 Aura bot, which was the original GN bot. This CL makes sure they run on the main Windows x86 and x64 bots now that they are also using GN. This CL also takes win8_chromium_ng out of the CQ since it's now redundant. R=brucedawson@chromium.org, msw@chromium.org BUG=605318 ========== to ========== Move remaining tests on win GN bots to main bots and update CQ There were a few test steps running only on the Win8 Aura bot, which was the original GN bot. This CL makes sure they run on the main Windows x86 and x64 bots now that they are also using GN. This CL also takes win8_chromium_ng out of the CQ since it's now redundant. R=brucedawson@chromium.org, msw@chromium.org BUG=605318 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Move remaining tests on win GN bots to main bots and update CQ There were a few test steps running only on the Win8 Aura bot, which was the original GN bot. This CL makes sure they run on the main Windows x86 and x64 bots now that they are also using GN. This CL also takes win8_chromium_ng out of the CQ since it's now redundant. R=brucedawson@chromium.org, msw@chromium.org BUG=605318 ========== to ========== Move remaining tests on win GN bots to main bots and update CQ There were a few test steps running only on the Win8 Aura bot, which was the original GN bot. This CL makes sure they run on the main Windows x86 and x64 bots now that they are also using GN. This CL also takes win8_chromium_ng out of the CQ since it's now redundant. R=brucedawson@chromium.org, msw@chromium.org BUG=605318 Committed: https://crrev.com/caa330a53f0d7972ae3fa276f0ade9f0e33e3037 Cr-Commit-Position: refs/heads/master@{#397982} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/caa330a53f0d7972ae3fa276f0ade9f0e33e3037 Cr-Commit-Position: refs/heads/master@{#397982} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
