zuul/zuul
Code Issues Proposed changes

Make github file annotation levels configurable via zuul return

Browse Source 
Change-Id: I4ccc264ce9b05d34e8456821b9cb85a28eaf5ed8
This commit is contained in:
Felix Edel 2020-03-03 16:24:59 +01:00
parent 2881ee5785
commit 431b4cfb57
No known key found for this signature in database
GPG Key ID: E95717A102DD3030
9 changed files with 84 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
doc/source/reference/jobs.rst
View File

@ -881,10 +881,12 @@ change, set the **zuul.file_comments** value. For example:
path/to/file.py:
- line: 42
message: "Line too long"
level: info
- line: 82
message: "Line too short"
- line: 119
message: "This block is indented too far."
level: warning
range:
start_line: 117
start_character: 0
@ -893,7 +895,8 @@ change, set the **zuul.file_comments** value. For example:
Not all reporters currently support line comments (or all of the
features of line comments); in these cases, reporters will simply
ignore this data.
ignore this data. The ``level`` is optional, but if provided must
be one of ``info``, ``warning``, ``error``.
Zuul will attempt to automatically translate the supplied line numbers
to the corresponding lines in the original change as written (they may

10
releasenotes/notes/file-comment-levels-9b26156addc3644f.yaml Normal file
View File

@ -0,0 +1,10 @@
---
features:
- |
The annotation levels for the file comments reported via Github checks
API are now configurable in ``zuul_return``. Each file comment entry can
provide an optional ``level`` parameter ``[info|warning|error]`` that will
be picked up by the Github reporter.
For more details on how to provide file comments from Zuul, see the
documentation of the :ref:`return_values`.

3
tests/fixtures/config/gerrit-file-comments/git/org_project/playbooks/file-comments.yaml vendored
View File

@ -9,6 +9,9 @@
message: line too long
- line: 82
message: line too short
- line: 2
message: levels are ignored by gerrit
level: warning
otherfile.txt:
- line: 21
message: |

3
tests/fixtures/config/github-file-comments/git/org_project/playbooks/file-comments-error.yaml vendored
View File

@ -19,6 +19,9 @@
- line: 7 # No message provided
- line: 9999
message: Line is not part of the file
- line: 3
message: Invalid level will fall back to warning
level: invalid-level
missingfile.txt:
- line: 1
message: "Missing file"

5
tests/fixtures/config/github-file-comments/git/org_project/playbooks/file-comments.yaml vendored
View File

@ -7,8 +7,12 @@
README:
- line: 1
message: "Simple line annotation"
- line: 2
message: "Line annotation with level"
level: warning
- line: 6
message: "simple range annotation"
level: info
range:
start_line: 4
start_character: 0
@ -16,6 +20,7 @@
end_character: 10
- line: 7
message: "Columns must be part of the same line"
level: error
range:
start_line: 7
start_character: 13

21
tests/unit/test_gerrit.py
View File

@ -246,9 +246,22 @@ class TestFileComments(AnsibleZuulTestCase):
'SUCCESS')
self.assertEqual(self.getJobFromHistory('file-comments-error').result,
'SUCCESS')
self.assertEqual(len(A.comments), 3)
self.assertEqual(len(A.comments), 4)
comments = sorted(A.comments, key=lambda x: x['line'])
self.assertEqual(comments[0],
self.assertEqual(
comments[0],
{
"file": "path/to/file.py",
"line": 2,
"message": "levels are ignored by gerrit",
"reviewer": {
"email": "zuul@example.com",
"name": "Zuul",
"username": "jenkins",
},
},
)
self.assertEqual(comments[1],
{'file': 'otherfile.txt',
'line': 21,
'message': 'This is a much longer message.\n\n'
@ -257,7 +270,7 @@ class TestFileComments(AnsibleZuulTestCase):
'name': 'Zuul',
'username': 'jenkins'}}
)
self.assertEqual(comments[1],
self.assertEqual(comments[2],
{'file': 'path/to/file.py',
'line': 42,
'message': 'line too long',
@ -265,7 +278,7 @@ class TestFileComments(AnsibleZuulTestCase):
'name': 'Zuul',
'username': 'jenkins'}}
)
self.assertEqual(comments[2],
self.assertEqual(comments[3],
{'file': 'path/to/file.py',
'line': 82,
'message': 'line too short',

28
tests/unit/test_github_driver.py
View File

@ -1777,7 +1777,7 @@ class TestCheckRunAnnotations(ZuulGithubAppTestCase, AnsibleZuulTestCase):
)
annotations = check_run["output"]["annotations"]
self.assertEqual(4, len(annotations))
self.assertEqual(6, len(annotations))
self.assertEqual(annotations[0], {
"path": "README",
@ -1787,19 +1787,27 @@ class TestCheckRunAnnotations(ZuulGithubAppTestCase, AnsibleZuulTestCase):
"end_line": 1,
})
# As the columns are not part of the same line, they are ignored in the
# annotation. Otherwise Github will complain about the request.
self.assertEqual(annotations[1], {
"path": "README",
"annotation_level": "warning",
"message": "Line annotation with level",
"start_line": 2,
"end_line": 2,
})
# As the columns are not part of the same line, they are ignored in the
# annotation. Otherwise Github will complain about the request.
self.assertEqual(annotations[2], {
"path": "README",
"annotation_level": "notice",
"message": "simple range annotation",
"start_line": 4,
"end_line": 6,
})
self.assertEqual(annotations[2], {
self.assertEqual(annotations[3], {
"path": "README",
"annotation_level": "warning",
"annotation_level": "failure",
"message": "Columns must be part of the same line",
"start_line": 7,
"end_line": 7,
@ -1810,10 +1818,18 @@ class TestCheckRunAnnotations(ZuulGithubAppTestCase, AnsibleZuulTestCase):
# From the invalid/error file comments, only the "line out of file"
# should remain. All others are excluded as they would result in
# invalid Github requests, making the whole check run update fail.
self.assertEqual(annotations[3], {
self.assertEqual(annotations[4], {
"path": "README",
"annotation_level": "warning",
"message": "Line is not part of the file",
"end_line": 9999,
"start_line": 9999
})
self.assertEqual(annotations[5], {
"path": "README",
"annotation_level": "warning",
"message": "Invalid level will fall back to warning",
"start_line": 3,
"end_line": 3,
})

25
zuul/driver/github/githubconnection.py
View File

@ -48,6 +48,16 @@ GITHUB_BASE_URL = 'https://api.github.com'
PREVIEW_JSON_ACCEPT = 'application/vnd.github.machine-man-preview+json'
PREVIEW_DRAFT_ACCEPT = 'application/vnd.github.shadow-cat-preview+json'
# NOTE (felix): Using log levels for file comments / annotations is IMHO more
# convenient than the values Github expects. Having in mind that those comments
# most probably come from various linters, "info", "warning" and "error"
# should be more general terms than "notice", "warning" and "failure".
ANNOTATION_LEVELS = {
"info": "notice",
"warning": "warning",
"error": "failure",
}
def _sign_request(body, secret):
signature = 'sha1=' + hmac.new(
@ -2011,19 +2021,20 @@ class GithubConnection(BaseConnection):
start_column = rng.get("start_character")
end_column = rng.get("end_character")
# TODO (felix): Make annotation_level configurable via
# file_comments in zuul_return. Other reporters like Gerrit
# might ignore the field if they don't support it.
# Accepted values are "notice", "warning", "failure".
# A "failure" annotation won't declare the check run as
# failure.
# Map the level coming from zuul_return to the ones Github
# expects. Each Github annotation must provide a level, so
# we fall back to "warning" in case no or an invalid level
# is provided.
annotation_level = ANNOTATION_LEVELS.get(
comment.get("level"), "warning"
)
# A Github check annotation requires at least the following
# attributes: "path", "start_line", "end_line", "message" and
# "annotation_level"
raw_annotation = {
"path": fn,
"annotation_level": "warning",
"annotation_level": annotation_level,
"message": comment["message"],
"start_line": start_line,
"end_line": end_line,

3
zuul/lib/filecomments.py
View File

@ -21,7 +21,8 @@ FILE_COMMENT = {
'start_character': int,
'end_line': int,
'end_character': int,
}
},
'level': str,
}
FILE_COMMENTS = {str: [FILE_COMMENT]}