Auto add the skip news label to docs PRs (#485)

* Remove unused import

* Refactor ``Category`` to be ``Labels``

* Rename from ``documentation`` to ``docs``

* Rename label

* Update comment

* Auto add the ``skip news`` label to ``docs`` PRs

Co-authored-by: Ezio Melotti <ezio.melotti@gmail.com>

* Apply suggestions from code review

Co-authored-by: Ezio Melotti <ezio.melotti@gmail.com>

* Fix test

* Update tests and coverage

* Remove auto-formatting by black

* Update bedevere/util.py

* Update bedevere/util.py

Co-authored-by: Ezio Melotti <ezio.melotti@gmail.com>

Co-authored-by: Ezio Melotti <ezio.melotti@gmail.com>

Author: DanielNoord

bedevere/prtype.py

@@ -17,13 +17,15 @@ class Labels(enum.Enum):
     performance = "performance"
     type_security = f"{TYPE_LABEL_PREFIX}-security"
     tests = "tests"
+    skip_news = "skip news"
 
 
-async def add_label(gh, issue, label):
-    """Apply this type label if there aren't any type labels on the PR."""
-    if any(label.startswith("type") for label in util.labels(issue)):
-        return
-    await gh.post(issue["labels_url"], data=[label.value])
+async def add_labels(gh, issue, labels):
+    """Add the specified labels to the PR."""
+    current_labels = util.labels(issue)
+    label_names = [c.value for c in labels if c.value not in current_labels]
+    if label_names:
+        await gh.post(issue["labels_url"], data=label_names)
 
 
 async def classify_by_filepaths(gh, pull_request, filenames):
@@ -35,10 +37,10 @@ async def classify_by_filepaths(gh, pull_request, filenames):
     The routing is handled by the filepaths module.
     """
     issue = await util.issue_for_PR(gh, pull_request)
-    docs = tests = False
+    news = docs = tests = False
     for filename in filenames:
         if util.is_news_dir(filename):
-            continue
+            news = True
         filepath = pathlib.PurePath(filename)
         if filepath.suffix == '.rst':
             docs = True
@@ -47,7 +49,10 @@ async def classify_by_filepaths(gh, pull_request, filenames):
         else:
             return
     if tests:
-        await add_label(gh, issue, Labels.tests)
+        await add_labels(gh, issue, [Labels.tests])
     elif docs:
-        await add_label(gh, issue, Labels.docs)
+        if news:
+            await add_labels(gh, issue, [Labels.docs])
+        else:
+            await add_labels(gh, issue, [Labels.docs, Labels.skip_news])
     return

bedevere/util.py

@@ -88,7 +88,8 @@ async def files_for_PR(gh, pull_request):
 
 
 async def issue_for_PR(gh, pull_request):
-    """Get the issue data for a pull request."""
+    """Return a dict with data about the given PR."""
+    # "issue_url" is the API endpoint for the given pull_request (despite the name)
     return await gh.getitem(pull_request["issue_url"])
 
 

tests/test_filepaths.py

@@ -88,7 +88,7 @@ async def test_docs_only(author_association):
     check_n_pop_nonews_events(gh, author_association == 'NONE')
     assert len(gh.post_url) == 1
     assert gh.post_url[0] == 'https://api.github.com/some/label'
-    assert gh.post_data[0] == [Labels.docs.value]
+    assert gh.post_data[0] == [Labels.docs.value, Labels.skip_news.value]
 
 
 @pytest.mark.parametrize('author_association', ['OWNER', 'MEMBER', 'CONTRIBUTOR', 'NONE'])

tests/test_prtype.py

@@ -85,7 +85,7 @@ async def test_docs_no_news():
     assert gh.getitem_url == 'https://api.github.com/repos/cpython/python/issue/1234'
     assert len(gh.post_url) == 1
     assert gh.post_url[0] == 'https://api.github.com/some/label'
-    assert gh.post_data[0] == [Labels.docs.value]
+    assert gh.post_data[0] == [Labels.docs.value, Labels.skip_news.value]
 
 
 async def test_docs_and_news():
@@ -154,7 +154,7 @@ async def test_docs_and_tests():
 
 async def test_leave_existing_type_labels():
     filenames = {'/path/to/docs.rst', 'test_docs2.py'}
-    issue = {'labels': [{'name': 'skip news'}, {'name': 'type-documentation'}],
+    issue = {'labels': [{'name': 'skip news'}, {'name': 'docs'}],
              'labels_url': 'https://api.github.com/some/label'}
     gh = FakeGH(getiter=filenames, getitem=issue)
     event_data = {
@@ -168,7 +168,29 @@ async def test_leave_existing_type_labels():
     }
     await prtype.classify_by_filepaths(gh, event_data['pull_request'], filenames)
     assert gh.getitem_url == 'https://api.github.com/repos/cpython/python/issue/1234'
-    # Only creates type-tests label.
+    assert len(gh.post_url) == 1
+    assert gh.post_url[0] == "https://api.github.com/some/label"
+    # This should only add the tests label as the docs label is already applied
+    assert gh.post_data[0] == [Labels.tests.value]
+
+
+async def test_do_not_post_if_nothing_to_apply():
+    filenames = {'/path/to/docs.rst'}
+    issue = {'labels': [{'name': 'skip news'}, {'name': 'docs'}],
+             'labels_url': 'https://api.github.com/some/label'}
+    gh = FakeGH(getiter=filenames, getitem=issue)
+    event_data = {
+        'action': 'opened',
+        'number': 1234,
+        'pull_request': {
+            'url': 'https://api.github.com/repos/cpython/python/pulls/1234',
+            'statuses_url': 'https://api.github.com/some/status',
+            'issue_url': 'https://api.github.com/repos/cpython/python/issue/1234',
+        },
+    }
+    await prtype.classify_by_filepaths(gh, event_data['pull_request'], filenames)
+    assert gh.getitem_url == 'https://api.github.com/repos/cpython/python/issue/1234'
+    # This should not post anything as docs is already applied
     assert len(gh.post_url) == 0