|
|
Created:
3 years, 8 months ago by Dirk Pranke Modified:
3 years, 8 months ago CC:
chromium-reviews Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionAttempt to disable tab_capture_end2end_tests on the trybots.
There is currently a weird interaction going on between
tab_capture_end2end_tests and browser_tests on the trybots, as the
result of some bugs in the `analyze` step. It's not immediately
obvious what the best way to fix this is, but it looks like
browser_tests isn't being run many times when it should be, and that's
bad. I think this CL will remove the collision and cause browser_tests
to run regularly, but it will cause the tab_capture tests to *not* run
on the trybots. Hopefully this is an acceptable temporary workaround,
but we need a real fix ASAP.
TBR=kbr@chromium.org, thakis@chromium.org, jam@chromium.org, enne@chromium.org
BUG=714336
NOTRY=true
Review-Url: https://codereview.chromium.org/2828143007
Cr-Commit-Position: refs/heads/master@{#466704}
Committed: https://chromium.googlesource.com/chromium/src/+/2b3b32f690472ea7bfb3c676adce4f3874bab95c
Patch Set 1 #Patch Set 2 : remove test.gni revert #
Messages
Total messages: 34 (21 generated)
The CQ bit was checked by dpranke@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Reverting the test.gni bit seems strange to me, end2end isn't a test(). Do you think renaming end2end back to _run and reverting the pyl file would help? On Apr 21, 2017 10:32 PM, <dpranke@chromium.org> wrote: > Reviewers: enne, jam, Ken Russell, Nico > CL: https://codereview.chromium.org/2828143007/ > > Description: > Attempt to disable tab_capture_end2end_tests on the trybots. > > There is currently a weird interaction going on between > tab_capture_end2end_tests and browser_tests on the trybots, as the > result of some bugs in the `analyze` step. It's not immediately > obvious what the best way to fix this is, but it looks like > browser_tests isn't being run many times when it should be, and that's > bad. I think this CL will remove the collision and cause browser_tests > to run regularly, but it will cause the tab_capture tests to *not* run > on the trybots. Hopefully this is an acceptable temporary workaround, > but we need a real fix ASAP. > > TBR=kbr@chromium.org, thakis@chromium.org, jam@chromium.org, > enne@chromium.org > BUG=714336 > > Affected files (+33, -5 lines): > M chrome/test/BUILD.gn > M testing/buildbot/gn_isolate_map.pyl > M testing/test.gni > M tools/mb/mb.py > > > Index: chrome/test/BUILD.gn > diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn > index 8e54d5edb58e330a3a7a9d49d7b2a37f6377b3bd.. > 9d026ee2bcb6c0f1cbe96e68ef6b03f934cb1107 100644 > --- a/chrome/test/BUILD.gn > +++ b/chrome/test/BUILD.gn > @@ -1136,12 +1136,17 @@ if (!is_android) { > ] > } > > - # TODO(GYP_GONE): Delete this after we've converted everything to GN. > - # This target exist only for compatibility w/ GYP. > + # TODO(dpranke), TODO(crbug.com/714336): tab_capture_end2end_tests > + # is really just browser_tests with some different flags. Figure out > + # what the right way to run this is given the different ways to > + # configure things in the //testing/buildbot/*.json files, here, > + # gn_isolate_map.pyl, and mb.py. There's too many knobs and the > interaction > + # is strange and unclear at this point. For now, I *think* this will just > + # have the effect of disabling the test in analyze. :(. > group("tab_capture_end2end_tests") { > testonly = true > deps = [ > - ":browser_tests", > + # ":browser_tests", > ] > } > } > Index: testing/buildbot/gn_isolate_map.pyl > diff --git a/testing/buildbot/gn_isolate_map.pyl > b/testing/buildbot/gn_isolate_map.pyl > index bb3f5f1c11cdabe06189bbe6324eaeaf6b842b3a.. > b5e3b0bd616a16cffa229379a8a029b1cb1aa772 100644 > --- a/testing/buildbot/gn_isolate_map.pyl > +++ b/testing/buildbot/gn_isolate_map.pyl > @@ -895,7 +895,9 @@ > "type": "additional_compile_target", > }, > "tab_capture_end2end_tests": { > - "label": "//chrome/test:browser_tests", > + # TODO(dpranke), TODO(crbug.com/714336): What is the right label to > + # have here (and/or the right way to run this?). > + "label": "//chrome/test:tab_capture_end2end_tests", > "type": "gpu_browser_test", > "gtest_filter": "CastStreamingApiTestWithPixelOutput.EndToEnd*: > TabCaptureApiPixelTest.EndToEnd*", > }, > Index: testing/test.gni > diff --git a/testing/test.gni b/testing/test.gni > index 96febe0f55e8f7a65668e09ad310e2ebea52034b.. > 407e8f45a050ee202c9c3e04713eeaede949ddcc 100644 > --- a/testing/test.gni > +++ b/testing/test.gni > @@ -283,6 +283,24 @@ template("test") { > "//build/win:default_exe_manifest", > ] > } > + > + # TODO(GYP_GONE): Delete this after we've converted everything to GN. > + # The _run targets exist only for compatibility with GYP. > + group("${target_name}_run") { > + testonly = true > + deps = [ > + ":${invoker.target_name}", > + ] > + } > + > + if (defined(invoker.output_name) && target_name != invoker.output_name) { > + group("${invoker.output_name}_run") { > + testonly = true > + deps = [ > + ":${invoker.target_name}", > + ] > + } > + } > } > } > > Index: tools/mb/mb.py > diff --git a/tools/mb/mb.py b/tools/mb/mb.py > index 1aea7a38416b442db9753cb495d00a2559470349.. > 39e78fa007fa3cdbf840d83e547df3e7d453791b 100755 > --- a/tools/mb/mb.py > +++ b/tools/mb/mb.py > @@ -1286,16 +1286,19 @@ class MetaBuildWrapper(object): > return 0 > > gn_inp = {} > - gn_inp['files'] = ['//' + f for f in inp['files'] if not > f.startswith('//')] > + gn_inp['files'] = sorted(['//' + f for f in inp['files'] > + if not f.startswith('//')]) > > isolate_map = self.ReadIsolateMap() > err, gn_inp['additional_compile_targets'] = self.MapTargetsToLabels( > isolate_map, inp['additional_compile_targets']) > + gn_inp['additional_compile_targets'].sort() > if err: > raise MBErr(err) > > err, gn_inp['test_targets'] = self.MapTargetsToLabels( > isolate_map, inp['test_targets']) > + gn_inp['test_targets'].sort() > if err: > raise MBErr(err) > labels_to_targets = {} > > > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
On 2017/04/22 02:48:57, Nico wrote: > Reverting the test.gni bit seems strange to me, end2end isn't a test(). Whoops, that wasn't supposed to be part of the CL (I reverted it while testing stuff, but you're right that it doesn't matter). > Do you think renaming end2end back to _run and reverting the pyl file would > help? It might, but part of the problem is that things didn't actually work correctly before the change either, if I'm currently understanding the code right. I think it's better to figure out how things should actually work and fix it, which I'll probably work on today and possibly land instead of this change.
The CQ bit was checked by dpranke@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
On 2017/04/22 18:25:44, Dirk Pranke wrote: > On 2017/04/22 02:48:57, Nico wrote: > > Reverting the test.gni bit seems strange to me, end2end isn't a test(). > > Whoops, that wasn't supposed to be part of the CL (I reverted it while testing > stuff, but > you're right that it doesn't matter). > > > Do you think renaming end2end back to _run and reverting the pyl file would > > help? > > It might, but part of the problem is that things didn't actually work correctly > before the > change either, if I'm currently understanding the code right. I think it's > better to > figure out how things should actually work and fix it, which I'll probably work > on today > and possibly land instead of this change. Rs-lgtm if you want to land this.
Description was changed from ========== Attempt to disable tab_capture_end2end_tests on the trybots. There is currently a weird interaction going on between tab_capture_end2end_tests and browser_tests on the trybots, as the result of some bugs in the `analyze` step. It's not immediately obvious what the best way to fix this is, but it looks like browser_tests isn't being run many times when it should be, and that's bad. I think this CL will remove the collision and cause browser_tests to run regularly, but it will cause the tab_capture tests to *not* run on the trybots. Hopefully this is an acceptable temporary workaround, but we need a real fix ASAP. TBR=kbr@chromium.org, thakis@chromium.org, jam@chromium.org, enne@chromium.org BUG=714336 ========== to ========== Attempt to disable tab_capture_end2end_tests on the trybots. There is currently a weird interaction going on between tab_capture_end2end_tests and browser_tests on the trybots, as the result of some bugs in the `analyze` step. It's not immediately obvious what the best way to fix this is, but it looks like browser_tests isn't being run many times when it should be, and that's bad. I think this CL will remove the collision and cause browser_tests to run regularly, but it will cause the tab_capture tests to *not* run on the trybots. Hopefully this is an acceptable temporary workaround, but we need a real fix ASAP. TBR=kbr@chromium.org, thakis@chromium.org, jam@chromium.org, enne@chromium.org BUG=714336 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
I think I have a better fix in https://codereview.chromium.org/2838473002/ that should make both test suites run where appropriate, so I think I can abandon this CL instead. I'm testing that one now to see how it goes.
On 2017/04/23 02:26:56, Dirk Pranke wrote: > I think I have a better fix in https://codereview.chromium.org/2838473002/ that > should > make both test suites run where appropriate, so I think I can abandon this CL > instead. > I'm testing that one now to see how it goes. Looks like tab_capture is hanging swarming for some reason, so my other CL with the "real fix" is actually worse. I'm reverting that, and I'll land this so that browser_tests will at least run properly.
The CQ bit was checked by dpranke@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org Link to the patchset: https://codereview.chromium.org/2828143007/#ps40001 (title: "Rework tab_capture_end2end_tests' test definition.")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
Patchset #3 (id:40001) has been deleted
The CQ bit was checked by dpranke@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by dpranke@chromium.org
The CQ bit was unchecked by dpranke@chromium.org
Description was changed from ========== Attempt to disable tab_capture_end2end_tests on the trybots. There is currently a weird interaction going on between tab_capture_end2end_tests and browser_tests on the trybots, as the result of some bugs in the `analyze` step. It's not immediately obvious what the best way to fix this is, but it looks like browser_tests isn't being run many times when it should be, and that's bad. I think this CL will remove the collision and cause browser_tests to run regularly, but it will cause the tab_capture tests to *not* run on the trybots. Hopefully this is an acceptable temporary workaround, but we need a real fix ASAP. TBR=kbr@chromium.org, thakis@chromium.org, jam@chromium.org, enne@chromium.org BUG=714336 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Attempt to disable tab_capture_end2end_tests on the trybots. There is currently a weird interaction going on between tab_capture_end2end_tests and browser_tests on the trybots, as the result of some bugs in the `analyze` step. It's not immediately obvious what the best way to fix this is, but it looks like browser_tests isn't being run many times when it should be, and that's bad. I think this CL will remove the collision and cause browser_tests to run regularly, but it will cause the tab_capture tests to *not* run on the trybots. Hopefully this is an acceptable temporary workaround, but we need a real fix ASAP. TBR=kbr@chromium.org, thakis@chromium.org, jam@chromium.org, enne@chromium.org BUG=714336 NOTRY=true ==========
bypassing the tryjobs for now, the failures are flakes.
The CQ bit was checked by dpranke@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493059828603420, "parent_rev": "5643cdec07486c2a894fd71318ab57fd0616f0fe", "commit_rev": "2b3b32f690472ea7bfb3c676adce4f3874bab95c"}
Message was sent while issue was closed.
Description was changed from ========== Attempt to disable tab_capture_end2end_tests on the trybots. There is currently a weird interaction going on between tab_capture_end2end_tests and browser_tests on the trybots, as the result of some bugs in the `analyze` step. It's not immediately obvious what the best way to fix this is, but it looks like browser_tests isn't being run many times when it should be, and that's bad. I think this CL will remove the collision and cause browser_tests to run regularly, but it will cause the tab_capture tests to *not* run on the trybots. Hopefully this is an acceptable temporary workaround, but we need a real fix ASAP. TBR=kbr@chromium.org, thakis@chromium.org, jam@chromium.org, enne@chromium.org BUG=714336 NOTRY=true ========== to ========== Attempt to disable tab_capture_end2end_tests on the trybots. There is currently a weird interaction going on between tab_capture_end2end_tests and browser_tests on the trybots, as the result of some bugs in the `analyze` step. It's not immediately obvious what the best way to fix this is, but it looks like browser_tests isn't being run many times when it should be, and that's bad. I think this CL will remove the collision and cause browser_tests to run regularly, but it will cause the tab_capture tests to *not* run on the trybots. Hopefully this is an acceptable temporary workaround, but we need a real fix ASAP. TBR=kbr@chromium.org, thakis@chromium.org, jam@chromium.org, enne@chromium.org BUG=714336 NOTRY=true Review-Url: https://codereview.chromium.org/2828143007 Cr-Commit-Position: refs/heads/master@{#466704} Committed: https://chromium.googlesource.com/chromium/src/+/2b3b32f690472ea7bfb3c676adce... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/2b3b32f690472ea7bfb3c676adce... |