Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2207263002:
Add logcats link in builbot page for android swarming tasks (Closed)
|
Created:
4 years, 4 months ago by nicholaslin Modified:
4 years, 4 months ago CC:
chromium-reviews, infra-reviews+build_chromium.org, kjellander-cc_chromium.org Base URL:
https://chromium.googlesource.com/chromium/tools/build.git@master Target Ref:
refs/heads/master Project:
build Visibility:
Public. |
DescriptionAdd logcats link in builbot page for android swarming tasks
BUG=chromium:448050
Committed: https://chromium.googlesource.com/chromium/tools/build/+/c9c68ee6bcf16f9b5e3b0a9a34e4720ebf9d0126
Patch Set 1 #
Total comments: 2
Patch Set 2 : Update hash, add check for logdog #
Total comments: 2
Patch Set 3 : Make logdog stuff into swarming links #Patch Set 4 : Change one docstring #Patch Set 5 : Delete unused import #Patch Set 6 : Fix pylint error #Patch Set 7 : Update expectations #
Total comments: 2
Patch Set 8 : Address comments. #Patch Set 9 : Address comments, rename tests #Patch Set 10 : Apply merge #Patch Set 11 : Fix mismatch links on task_id #
Messages
Total messages: 47 (30 generated)
nicholaslin@google.com changed reviewers: + martiniss@chromium.org, stip@chromium.org
Ptal
https://codereview.chromium.org/2207263002/diff/1/scripts/slave/recipe_module... File scripts/slave/recipe_modules/chromium_tests/steps.py (right): https://codereview.chromium.org/2207263002/diff/1/scripts/slave/recipe_module... scripts/slave/recipe_modules/chromium_tests/steps.py:920: if hasattr(task, 'trigger_output') and 'tasks' in task.trigger_output: If we don't have this, what do we do? Is it an error? I would think so, right? We should handle that, and output something like "logcat missing, file a bug?" or something. Does that make sense?
ptal https://codereview.chromium.org/2207263002/diff/1/scripts/slave/recipe_module... File scripts/slave/recipe_modules/chromium_tests/steps.py (right): https://codereview.chromium.org/2207263002/diff/1/scripts/slave/recipe_module... scripts/slave/recipe_modules/chromium_tests/steps.py:920: if hasattr(task, 'trigger_output') and 'tasks' in task.trigger_output: On 2016/08/04 00:44:09, martiniss wrote: > If we don't have this, what do we do? Is it an error? I would think so, right? > We should handle that, and output something like "logcat missing, file a bug?" > or something. Does that make sense? Added in an additional check to make sure the logdog butler cipd package is installed. If the butler package is present and there is an output I think its safe to assume the task is written to logdog?
https://codereview.chromium.org/2207263002/diff/20001/scripts/slave/recipe_mo... File scripts/slave/recipe_modules/chromium_tests/steps.py (right): https://codereview.chromium.org/2207263002/diff/20001/scripts/slave/recipe_mo... scripts/slave/recipe_modules/chromium_tests/steps.py:916: # If Android swarming tasks, create link for unified logcats I wonder if there is a way to make this more generic so it's not just an android specific hack. What I'm thinking about is a way to specify a 'link template' in the buildbot json. We'd read that here and fill in the template if it exists. 'gtest_tests': [ { 'test': 'chrome_public_test_apk', 'swarming': { blah }, 'post_run_links': { 'shard_index: ${SHARD_INDEX} logcats': 'https://luci-logdog.appspot.com/v/android/swarming/logcats/${TASK_ID}/+/unified_logcats' } } ] maybe that's too crazy though
https://codereview.chromium.org/2207263002/diff/20001/scripts/slave/recipe_mo... File scripts/slave/recipe_modules/chromium_tests/steps.py (right): https://codereview.chromium.org/2207263002/diff/20001/scripts/slave/recipe_mo... scripts/slave/recipe_modules/chromium_tests/steps.py:916: # If Android swarming tasks, create link for unified logcats On 2016/08/04 23:55:58, stip wrote: > I wonder if there is a way to make this more generic so it's not just an android > specific hack. What I'm thinking about is a way to specify a 'link template' in > the buildbot json. We'd read that here and fill in the template if it exists. > > 'gtest_tests': [ > { > 'test': 'chrome_public_test_apk', > 'swarming': { blah }, > 'post_run_links': { > 'shard_index: ${SHARD_INDEX} logcats': > 'https://luci-logdog.appspot.com/v/android/swarming/logcats/${TASK_ID}/+/unified_logcats' > } > } > ] > > maybe that's too crazy though Perhaps. Maybe not a link template but maybe a logdog_url flag or something? Then the android check can be replaced by a logdog_url check. Also we would no longer have to check for cipd_packages.
ptal
lgtm w/ changes https://codereview.chromium.org/2207263002/diff/120001/scripts/slave/recipe_m... File scripts/slave/recipe_modules/chromium_tests/steps.py (right): https://codereview.chromium.org/2207263002/diff/120001/scripts/slave/recipe_m... scripts/slave/recipe_modules/chromium_tests/steps.py:928: link = output['link'] can you move this down so it's just above 931? that way all the name stuff is together and the link stuff is together as well https://codereview.chromium.org/2207263002/diff/120001/scripts/slave/recipe_m... scripts/slave/recipe_modules/chromium_tests/steps.py:931: link = ''.join(link) put a comment here on why we're doing this
The CQ bit was checked by nicholaslin@google.com 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: This issue passed the CQ dry run.
The CQ bit was checked by nicholaslin@google.com 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: Recipe Roll Downstream Tester on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/3094bb01a738cd10)
The CQ bit was checked by nicholaslin@google.com 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: Build Presubmit on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/3094e0e947b5e210)
The CQ bit was checked by nicholaslin@google.com 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: This issue passed the CQ dry run.
The CQ bit was checked by nicholaslin@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from stip@chromium.org Link to the patchset: https://codereview.chromium.org/2207263002/#ps180001 (title: "Apply merge")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
nicholaslin@google.com changed reviewers: + agable@chromium.org
ptal
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Build Presubmit on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/309549d33914b810)
nicholaslin@google.com changed reviewers: + phajdan.jr@chromium.org - agable@chromium.org
phajdan: ptal OWNERS
Description was changed from ========== Add logcats link in builbot page for android swarming tasks BUG=chromium:448050 ========== to ========== Add logcats link in builbot page for android swarming tasks BUG=chromium:448050 ==========
stip@chromium.org changed reviewers: + sergeyberezin@chromium.org
sergeyberezin: ptal OWNERS
rs lgtm
rs lgtm
The CQ bit was checked by nicholaslin@google.com 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: This issue passed the CQ dry run.
The CQ bit was checked by nicholaslin@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Add logcats link in builbot page for android swarming tasks BUG=chromium:448050 ========== to ========== Add logcats link in builbot page for android swarming tasks BUG=chromium:448050 Committed: https://chromium.googlesource.com/chromium/tools/build/+/c9c68ee6bcf16f9b5e3b... ==========
Message was sent while issue was closed.
Committed patchset #10 (id:180001) as https://chromium.googlesource.com/chromium/tools/build/+/c9c68ee6bcf16f9b5e3b...
Message was sent while issue was closed.
A revert of this CL (patchset #10 id:180001) has been created in https://codereview.chromium.org/2248443002/ by nicholaslin@google.com. The reason for reverting is: populating empty links.. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
