From 986505f4893ae4c0cd11f0f2a6ea4a2cc27f9488 Mon Sep 17 00:00:00 2001 From: Debrian Figueroa Date: Wed, 10 Jul 2019 18:20:20 +0000 Subject: [PATCH] Updated error message format Added headers and code tags Made sure that when limiting message size, at least part of the message is seen. Change-Id: I404e2f7e86907f701332a62fe46334ff437cca03 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1690741 Reviewed-by: Stephen Martinis Reviewed-by: Robbie Iannucci Commit-Queue: Debrian Figueroa --- recipes/recipe_modules/presubmit/api.py | 31 +++++++------ .../recipe_modules/presubmit/tests/execute.py | 44 +++++++------------ 2 files changed, 32 insertions(+), 43 deletions(-) diff --git a/recipes/recipe_modules/presubmit/api.py b/recipes/recipe_modules/presubmit/api.py index 166bebc37..0d7fa8981 100644 --- a/recipes/recipe_modules/presubmit/api.py +++ b/recipes/recipe_modules/presubmit/api.py @@ -164,18 +164,21 @@ def _limitSize(message_list, char_limit=450): for index, message in enumerate(message_list): char_count += len(message) if char_count > char_limit: - total_errors = len(message_list) - oversized_msg = ('**Error size > %d chars, ' - 'there are %d more error(s) (%d total)**') % ( - char_limit, total_errors - index, total_errors - ) if index == 0: # Show at minimum part of the first error message - first_message = message_list[index].replace('\n\n', '\n') - return ['\n\n'.join( - _limitSize(first_message.splitlines()) + first_message = message_list[index].splitlines() + return ['\n'.join( + _limitSize(first_message) ) ] + total_errors = len(message_list) + # If code is being cropped, the closing code tag will + # get removed, so add it back before the hint. + code_tag = '```' + oversized_msg = ('%s\n**Error size > %d chars, ' + 'there are %d more error(s) (%d total)**') % ( + code_tag, char_limit, total_errors - index, total_errors + ) return message_list[:index] + [oversized_msg, hint] return message_list @@ -220,20 +223,16 @@ def _createSummaryMarkdown(step_json): warning_count = len(step_json['warnings']) notif_count = len(step_json['notifications']) description = ( - 'There are %d error(s), %d warning(s),' + '### There are %d error(s), %d warning(s),' ' and %d notifications(s). Here are the errors:') % ( len(errors), warning_count, notif_count ) error_messages = [] for error in errors: - # markdown represents new lines with 2 spaces - # replacing the \n with \n\n because \n gets replaced with an empty space. - # This way it will work on both markdown and plain text. error_messages.append( - '**ERROR**\n\n%s\n\n%s' % ( - error['message'].replace('\n', '\n\n'), - error['long_text'].replace('\n', '\n\n')) + '**ERROR**\n```\n%s\n%s\n```' % ( + error['message'], error['long_text']) ) error_messages = _limitSize(error_messages) @@ -242,7 +241,7 @@ def _createSummaryMarkdown(step_json): error_messages.insert(0, description) if warning_count or notif_count: error_messages.append( - ('To see notifications and warnings,' + ('##### To see notifications and warnings,' ' look at the stdout of the presubmit step.') ) return '\n\n'.join(error_messages) diff --git a/recipes/recipe_modules/presubmit/tests/execute.py b/recipes/recipe_modules/presubmit/tests/execute.py index 863c296ba..0fb0984b0 100644 --- a/recipes/recipe_modules/presubmit/tests/execute.py +++ b/recipes/recipe_modules/presubmit/tests/execute.py @@ -78,7 +78,7 @@ def GenTests(api): api.post_process(post_process.StatusFailure) + api.post_process( post_process.ResultReason, - (u'There are 0 error(s), 0 warning(s), and 0 notifications(s).' + (u'### There are 0 error(s), 0 warning(s), and 0 notifications(s).' ' Here are the errors:' '\n\nTimeout occurred during presubmit step.')) + api.post_process(post_process.DropExpectation) @@ -124,21 +124,21 @@ def GenTests(api): ) + api.post_process(post_process.StatusFailure) + api.post_process(post_process.ResultReason, textwrap.dedent(u''' - There are 2 error(s), 1 warning(s), and 1 notifications(s). Here are the errors: + ### There are 2 error(s), 1 warning(s), and 1 notifications(s). Here are the errors: **ERROR** - + ``` Missing LGTM - Here are some suggested OWNERS: fake@ + ``` **ERROR** - + ``` Syntax error in fake.py - Expected "," after item in list + ``` - To see notifications and warnings, look at the stdout of the presubmit step. + ##### To see notifications and warnings, look at the stdout of the presubmit step. ''').strip() ) + api.post_process(post_process.DropExpectation) @@ -166,34 +166,23 @@ def GenTests(api): ) + api.post_process(post_process.StatusFailure) + api.post_process(post_process.ResultReason, textwrap.dedent(''' - There are 1 error(s), 0 warning(s), and 0 notifications(s). Here are the errors: + ### There are 1 error(s), 0 warning(s), and 0 notifications(s). Here are the errors: **ERROR** - + ``` Missing LGTM - Here are some suggested OWNERS: - reallyLongFakeAccountNameEmail@chromium.org - reallyLongFakeAccountNameEmail@chromium.org - reallyLongFakeAccountNameEmail@chromium.org - reallyLongFakeAccountNameEmail@chromium.org - reallyLongFakeAccountNameEmail@chromium.org - reallyLongFakeAccountNameEmail@chromium.org - reallyLongFakeAccountNameEmail@chromium.org - reallyLongFakeAccountNameEmail@chromium.org - reallyLongFakeAccountNameEmail@chromium.org - - **Error size > 450 chars, there are 1 more error(s) (13 total)** - + ``` + **Error size > 450 chars, there are 2 more error(s) (15 total)** **The complete output can be found at the bottom of the presubmit stdout.** ''').strip() ) + @@ -220,13 +209,14 @@ def GenTests(api): ) + api.post_process(post_process.StatusFailure) + api.post_process(post_process.ResultReason, textwrap.dedent(u''' - There are 1 error(s), 0 warning(s), and 0 notifications(s). Here are the errors: + ### There are 1 error(s), 0 warning(s), and 0 notifications(s). Here are the errors: - **ERROR** + **ERROR** + ``` + Infra Failure - Infra Failure - - ''').lstrip() + ``` + ''').strip() ) + api.post_process(post_process.DropExpectation) )