chore: add labels with context info (#518)

so it will be easy to collect statistics about PRs with context

Author: SurferJeffAtGoogle

autosynth/change_pusher.py

@@ -13,15 +13,24 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import json
 import os
-
+import subprocess
+import tempfile
+import typing
+import uuid
 from abc import ABC, abstractmethod
+
 from autosynth import git, github
 from autosynth.log import logger
-import subprocess
-import uuid
-import tempfile
+
+
+class AbstractPullRequest(ABC):
+    """Abstractly, manipulates an existing pull request."""
+
+    @abstractmethod
+    def add_labels(self, labels: typing.Sequence[str]) -> None:
+        """Adds labels to an existing pull request."""
+        pass
 
 
 class AbstractChangePusher(ABC):
@@ -30,15 +39,37 @@ class AbstractChangePusher(ABC):
     @abstractmethod
     def push_changes(
         self, commit_count: int, branch: str, pr_title: str, synth_log: str = ""
-    ) -> None:
-        """Creates a pull request from commits in current working directory."""
+    ) -> AbstractPullRequest:
+        """Creates a pull request from commits in current working directory.
+
+        Arguments:
+            commit_count {int} -- How many commits are in this pull request?
+            branch {str} -- The name of the local branch to push.
+            pr_title {str} -- The title for the pull request.
+
+        Keyword Arguments:
+            synth_log {str} -- The full log of the call to synth. (default: {""})
+
+        Returns:
+            A pull request.
+        """
         pass
 
     @abstractmethod
     def check_if_pr_already_exists(self, branch) -> bool:
         pass
 
 
+class PullRequest(AbstractPullRequest):
+    def __init__(self, gh: github.GitHub, pr: typing.Dict[str, typing.Any]):
+        self._gh = gh
+        self._pr = pr
+
+    def add_labels(self, labels: typing.Sequence[str]) -> None:
+        """Adds labels to an existing pull request."""
+        self._gh.update_pull_labels(self._pr, add=labels)
+
+
 class ChangePusher(AbstractChangePusher):
     """Actually pushes changes to github."""
 
@@ -49,7 +80,7 @@ def __init__(self, repository: str, gh: github.GitHub, synth_path: str):
 
     def push_changes(
         self, commit_count: int, branch: str, pr_title: str, synth_log: str = ""
-    ) -> None:
+    ) -> AbstractPullRequest:
         git.push_changes(branch)
 
         pr = self._gh.create_pull_request(
@@ -64,7 +95,8 @@ def push_changes(
             api_label = self._gh.get_api_label(self._repository, self._synth_path)
 
             if api_label:
-                self._gh.update_pull_labels(json.loads(pr), add=[api_label])
+                self._gh.update_pull_labels(pr, add=[api_label])
+        return PullRequest(self._gh, pr)
 
     def check_if_pr_already_exists(self, branch) -> bool:
         repo = self._repository
@@ -87,13 +119,12 @@ def __init__(self, inner_change_pusher: AbstractChangePusher):
 
     def push_changes(
         self, commit_count: int, branch: str, pr_title: str, synth_log: str = ""
-    ) -> None:
+    ) -> AbstractPullRequest:
         if commit_count < 2:
             # Only one change, no need to squash.
-            self.inner_change_pusher.push_changes(
+            return self.inner_change_pusher.push_changes(
                 commit_count, branch, pr_title, synth_log
             )
-            return
 
         subprocess.check_call(["git", "checkout", branch])  # Probably redundant.
         with tempfile.NamedTemporaryFile() as message_file:
@@ -111,7 +142,7 @@ def push_changes(
             subprocess.check_call(["git", "checkout", "-b", branch])
             subprocess.check_call(["git", "merge", "--squash", temp_branch])
             subprocess.check_call(["git", "commit", "-F", message_file.name])
-        self.inner_change_pusher.push_changes(1, branch, pr_title, synth_log)
+        return self.inner_change_pusher.push_changes(1, branch, pr_title, synth_log)
 
     def check_if_pr_already_exists(self, branch) -> bool:
         return self.inner_change_pusher.check_if_pr_already_exists(branch)

autosynth/github.py

@@ -16,7 +16,7 @@
 from typing import Generator, Sequence, Dict, Optional
 
 import requests
-
+import typing
 
 _GITHUB_ROOT: str = "https://api.github.com"
 
@@ -50,7 +50,7 @@ def list_pull_requests(self, repository: str, **kwargs) -> Sequence[Dict]:
 
     def create_pull_request(
         self, repository: str, branch: str, title: str, body: str = None
-    ) -> str:
+    ) -> typing.Dict[str, typing.Any]:
         url = f"{_GITHUB_ROOT}/repos/{repository}/pulls"
         response = self.session.post(
             url,

autosynth/synth.py

@@ -283,7 +283,15 @@ def push_changes(self, change_pusher: AbstractChangePusher) -> None:
         pr_title = _compose_pr_title(
             self.count_commits_with_context(), self._synth_path, self.source_name
         )
-        change_pusher.push_changes(self.commit_count, self.branch, pr_title)
+        pr = change_pusher.push_changes(self.commit_count, self.branch, pr_title)
+        # Add a label to make it easy to collect statistics about commits with context.
+        if self.count_commits_with_context() == 0:
+            label = "context: none"
+        elif self.count_commits_with_context() == self.commit_count:
+            label = "context: full"
+        else:
+            label = "context: partial"
+        pr.add_labels([label])
 
 
 def _compose_pr_title(

tests/test_synth.py

@@ -27,7 +27,11 @@
 import autosynth.abstract_source
 import autosynth.synth
 from autosynth import git
-from autosynth.change_pusher import AbstractChangePusher, SquashingChangePusher
+from autosynth.change_pusher import (
+    AbstractChangePusher,
+    SquashingChangePusher,
+    AbstractPullRequest,
+)
 from autosynth.synthesizer import AbstractSynthesizer
 from integration_tests import util
 import json
@@ -163,11 +167,16 @@ def test_synthesize_loop_with_empty_change_history():
         assert 0 == commit_count
 
 
+class MockPullRequest(AbstractPullRequest):
+    def add_labels(self, labels: typing.Sequence[str]) -> None:
+        pass
+
+
 class MockChangePusher(AbstractChangePusher):
     def push_changes(
         self, commit_count: int, branch: str, pr_title: str = "", synth_log: str = ""
     ) -> None:
-        pass
+        return MockPullRequest()
 
     def check_if_pr_already_exists(self, branch) -> bool:
         return False
@@ -235,12 +244,14 @@ def test_synthesize_loop_with_realistic_change_history_multiple_prs(
             "test-source1",
             "[CHANGE ME] Re-generated to pick up changes from source1.",
         ),
+        call.push_changes().add_labels(["context: partial"]),
         call.check_if_pr_already_exists("test-source2"),
         call.push_changes(
             3,
             "test-source2",
             "[CHANGE ME] Re-generated to pick up changes from source2.",
         ),
+        call.push_changes().add_labels(["context: partial"]),
     ]
     assert golden_calls == calls
 
@@ -262,13 +273,15 @@ def test_synthesize_loop_with_realistic_change_history_squash_prs(
             "[CHANGE ME] Re-generated to pick up changes from source1.",
             "",
         ),
+        call.push_changes().add_labels(["context: partial"]),
         call.check_if_pr_already_exists("test-source2"),
         call.push_changes(
             1,
             "test-source2",
             "[CHANGE ME] Re-generated to pick up changes from source2.",
             "",
         ),
+        call.push_changes().add_labels(["context: partial"]),
     ]
     assert golden_calls == calls
 
@@ -426,6 +439,7 @@ def test_synthesize_loop_uses_single_commit_title_for_pr_title(
         call.check_if_pr_already_exists("test-source1"),
         call.check_if_pr_already_exists("test-source2"),
         call.push_changes(1, "test-source2", "Wrote a to a.txt."),
+        call.push_changes().add_labels(["context: full"]),
     ]
     assert golden_calls == calls