Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1864933002:
Add an actionable error message when recipe_simulation_test.py fails (Closed)
DescriptionAdd an actionable error message when recipe_simulation_test.py fails
Describes how to create a coverage report, and how to re-train.
BUG=none
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299804
Patch Set 1 #Patch Set 2 : add todo #Patch Set 3 : rebase #Messages
Total messages: 26 (11 generated)
agrieve@chromium.org changed reviewers: + dpranke@chromium.org
Spent a good 30 minutes figuring out how to create a coverage report. Hoping to save the next person the trouble :)
dpranke@chromium.org changed reviewers: + iannucci@chromium.org
lgtm, but it's possible there's a reason there was an os.execvp there besides unnecessary optimization -- :) -- so cc'ing iannucci as well.
iannucci@google.com changed reviewers: + iannucci@google.com, martiniss@chromium.org
lgtm, though I wonder if this should really be done upstream in github.com/luci/recipes-py/? martiniss, wdyt?
On 2016/04/06 01:54:27, iannucci1 wrote: > lgtm, though I wonder if this should really be done upstream in > github.com/luci/recipes-py/? martiniss, wdyt? Yeah, my concern was that it'd be getting away from the actual binary that was run (want it to print "scripts/slave/unittests/recipe_simulation_test.py")
On 2016/04/06 01:56:32, agrieve wrote: > On 2016/04/06 01:54:27, iannucci1 wrote: > > lgtm, though I wonder if this should really be done upstream in > > github.com/luci/recipes-py/? martiniss, wdyt? > > Yeah, my concern was that it'd be getting away from the actual binary that was > run (want it to print "scripts/slave/unittests/recipe_simulation_test.py") Ok. Then I think for now it would be fine to commit as-is, but I'd like to get martiniss' thoughts. We have some Q2 OKRs to do some cleanup in the whole recipes/docs/tools/repos/wtf-is-happening department this quarter. I suspect we'll move this upstream at that point.
On 2016/04/06 02:04:43, iannucci wrote: > On 2016/04/06 01:56:32, agrieve wrote: > > On 2016/04/06 01:54:27, iannucci1 wrote: > > > lgtm, though I wonder if this should really be done upstream in > > > github.com/luci/recipes-py/? martiniss, wdyt? > > > > Yeah, my concern was that it'd be getting away from the actual binary that was > > run (want it to print "scripts/slave/unittests/recipe_simulation_test.py") > > Ok. Then I think for now it would be fine to commit as-is, but I'd like to get > martiniss' thoughts. > > We have some Q2 OKRs to do some cleanup in the whole > recipes/docs/tools/repos/wtf-is-happening department this quarter. I suspect > we'll move this upstream at that point. I'm happy to wait for martiniss. There's no urgency to this.
On 2016/04/06 at 02:14:19, agrieve wrote: > On 2016/04/06 02:04:43, iannucci wrote: > > On 2016/04/06 01:56:32, agrieve wrote: > > > On 2016/04/06 01:54:27, iannucci1 wrote: > > > > lgtm, though I wonder if this should really be done upstream in > > > > github.com/luci/recipes-py/? martiniss, wdyt? > > > > > > Yeah, my concern was that it'd be getting away from the actual binary that was > > > run (want it to print "scripts/slave/unittests/recipe_simulation_test.py") > > > > Ok. Then I think for now it would be fine to commit as-is, but I'd like to get > > martiniss' thoughts. > > > > We have some Q2 OKRs to do some cleanup in the whole > > recipes/docs/tools/repos/wtf-is-happening department this quarter. I suspect > > we'll move this upstream at that point. > > I'm happy to wait for martiniss. There's no urgency to this. I'm fine with landing this for now. I'll make a new bug (https://bugs.chromium.org/p/chromium/issues/detail?id=601662) about documenting this; i like your idea of printing something if the simulation tests fail. Could you add a todo for me for that?
(lgtm :)
On 2016/04/08 01:05:13, martiniss wrote: > On 2016/04/06 at 02:14:19, agrieve wrote: > > On 2016/04/06 02:04:43, iannucci wrote: > > > On 2016/04/06 01:56:32, agrieve wrote: > > > > On 2016/04/06 01:54:27, iannucci1 wrote: > > > > > lgtm, though I wonder if this should really be done upstream in > > > > > github.com/luci/recipes-py/? martiniss, wdyt? > > > > > > > > Yeah, my concern was that it'd be getting away from the actual binary that > was > > > > run (want it to print "scripts/slave/unittests/recipe_simulation_test.py") > > > > > > Ok. Then I think for now it would be fine to commit as-is, but I'd like to > get > > > martiniss' thoughts. > > > > > > We have some Q2 OKRs to do some cleanup in the whole > > > recipes/docs/tools/repos/wtf-is-happening department this quarter. I suspect > > > we'll move this upstream at that point. > > > > I'm happy to wait for martiniss. There's no urgency to this. > > I'm fine with landing this for now. I'll make a new bug > (https://bugs.chromium.org/p/chromium/issues/detail?id=601662) about documenting > this; i like your idea of printing something if the simulation tests fail. Could > you add a todo for me for that? Done
The CQ bit was checked by agrieve@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dpranke@chromium.org, martiniss@chromium.org, iannucci@google.com Link to the patchset: https://codereview.chromium.org/1864933002/#ps20001 (title: "add todo")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1864933002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1864933002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Build Presubmit on tryserver.infra (JOB_FAILED, https://build.chromium.org/p/tryserver.infra/builders/Build%20Presubmit/build...)
The CQ bit was checked by agrieve@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dpranke@chromium.org, martiniss@chromium.org, iannucci@google.com Link to the patchset: https://codereview.chromium.org/1864933002/#ps40001 (title: "rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1864933002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1864933002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Build Presubmit on tryserver.infra (JOB_FAILED, https://build.chromium.org/p/tryserver.infra/builders/Build%20Presubmit/build...)
The CQ bit was checked by agrieve@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1864933002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1864933002/40001
Message was sent while issue was closed.
Description was changed from ========== Add an actionable error message when recipe_simulation_test.py fails Describes how to create a coverage report, and how to re-train. BUG=none ========== to ========== Add an actionable error message when recipe_simulation_test.py fails Describes how to create a coverage report, and how to re-train. BUG=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299804 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as http://src.chromium.org/viewvc/chrome?view=rev&revision=299804 |
