[1/n][sl][github][gh stacks] support headers and list params

eden/scm/sapling/ext/github/github_gh_cli.py

@@ -16,32 +16,43 @@
 
 JsonDict = Dict[str, Any]
 
+# Scalar value that can be passed as a field to `gh api`.
+_ScalarParam = Union[str, int, bool]
+# `gh api` also supports array fields via repeated `key[]=value` args.
+ParamValue = Union[_ScalarParam, List[_ScalarParam]]
+
 
 async def make_request(
-    params: Dict[str, Union[str, int, bool]],
+    params: Dict[str, ParamValue],
     hostname: str,
     endpoint="graphql",
     method: Optional[str] = None,
+    headers: Optional[Dict[str, str]] = None,
 ) -> Result[JsonDict, str]:
     """If successful, returns a Result whose value is parsed JSON returned by
     the request.
     """
-    return await _make_request(params, hostname, endpoint, method)
+    return await _make_request(params, hostname, endpoint, method, headers)
 
 
 # Unexported extension/mock point.
 async def _make_request(
-    params: Dict[str, Union[str, int, bool]],
+    params: Dict[str, ParamValue],
     hostname: str,
     endpoint: str,
     method: Optional[str],
+    headers: Optional[Dict[str, str]] = None,
 ) -> Result[JsonDict, str]:
     if method:
         endpoint_args = ["-X", method.upper(), endpoint]
     else:
         endpoint_args = [endpoint]
+    header_args = list(
+        itertools.chain(*[["-H", f"{k}: {v}"] for (k, v) in (headers or {}).items()])
+    )
     args = (
         ["gh", "api", "--hostname", hostname]
+        + header_args
         + endpoint_args
         + list(itertools.chain(*[_format_param(k, v) for (k, v) in params.items()]))
     )
@@ -82,7 +93,38 @@ async def _make_request(
         )
 
 
-def _format_param(key: str, value: Union[str, int, bool]) -> List[str]:
+def _format_param(key: str, value: ParamValue) -> List[str]:
+    r"""Formats a param as a list of arguments to pass to `gh api`.
+
+    >>> _format_param("body", "hello")
+    ['-f', 'body=hello']
+    >>> _format_param("number", 42)
+    ['-F', 'number=42']
+    >>> _format_param("draft", True)
+    ['-F', 'draft=true']
+
+    Array values use the `gh api` repeated-field syntax, e.g.
+    `-F "pull_requests[]=101" -F "pull_requests[]=102"`:
+
+    >>> _format_param("pull_requests", [101, 102])
+    ['-F', 'pull_requests[]=101', '-F', 'pull_requests[]=102']
+    >>> _format_param("labels", ["bug", "help wanted"])
+    ['-f', 'labels[]=bug', '-f', 'labels[]=help wanted']
+
+    An empty array is passed explicitly as `key[]` without a value (per the
+    `gh api` manual) rather than dropped: an empty array and a missing field
+    can have different meanings to an endpoint:
+
+    >>> _format_param("empty", [])
+    ['-F', 'empty[]']
+    """
+    if isinstance(value, list):
+        if not value:
+            return ["-F", f"{key}[]"]
+        return list(
+            itertools.chain(*[_format_param(f"{key}[]", v) for v in value])
+        )
+
     # In Python, bool is a subclass of int, so check it first.
     if isinstance(value, bool):
         opt = "-F"

eden/scm/sapling/ext/github/mock_utils.py

@@ -28,8 +28,11 @@
 REPO_ID = "R_test_github_repo"
 USER_NAME = "facebook_username"
 
-ParamsType = Dict[str, Union[bool, int, str]]
-MakeRequestType = Callable[[ParamsType, str, str, Optional[str]], Result[JsonDict, str]]
+ParamsType = Dict[str, Union[bool, int, str, List[bool], List[int], List[str]]]
+MakeRequestType = Callable[
+    [ParamsType, str, str, Optional[str], Optional[Dict[str, str]]],
+    Result[JsonDict, str],
+]
 RunGitCommandType = Callable[[List[str], str], bytes]
 
 
@@ -75,6 +78,7 @@ class MockGitHubServer:
     def __init__(self, hostname: str = GITHUB_HOSTNAME):
         self.hostname: str = hostname
         self.requests: Dict[str, MockRequest] = {}
+        self._consumed_keys: set = set()
 
     async def make_request(
         self,
@@ -83,10 +87,15 @@ async def make_request(
         hostname: str,
         endpoint: str = "graphql",
         method: Optional[str] = None,
+        headers: Optional[Dict[str, str]] = None,
     ) -> Result[JsonDict, str]:
         """Wrapper function for `github_gh_cli.make_request`.
 
         It reads mock data from `self.requests` instead of sending network requests.
+
+        Note that `headers` is intentionally not part of the request key: the
+        headers we send (e.g., X-GitHub-Api-Version) do not affect which mock
+        response should be returned.
         """
         assert real_make_request.__name__ == "_make_request", (
             f"expected '_make_request', but got '{real_make_request.__name__}'"
@@ -96,11 +105,45 @@ async def make_request(
 
         if key not in self.requests:
             raise MockRequestNotFound(key, self.requests)
+        self._consumed_keys.add(key)
         return self.requests[key].get_response()
 
     def _add_request(self, request_key: str, request: "MockRequest") -> None:
         self.requests[request_key] = request
 
+    def unconsumed_requests(self) -> List[str]:
+        """Keys of expectations that were never requested."""
+        return sorted(k for k in self.requests if k not in self._consumed_keys)
+
+    def report_unconsumed(self, ui) -> None:
+        """Prints a warning for every expectation that was never requested.
+
+        Intended to be called after the command under test has finished (see
+        wrap_with_consumption_check). Tests that use this produce no extra
+        output when all expectations were consumed, so any warning makes the
+        test fail: this catches code paths that silently stopped making a
+        request the test author expected.
+        """
+        for key in self.unconsumed_requests():
+            first_line = key.splitlines()[0]
+            ui.status_err(f"warning, unconsumed mock request: {first_line}\n")
+
+
+def wrap_with_consumption_check(server: "MockGitHubServer", module, funcname) -> None:
+    """Wraps `module.funcname` (a command function taking `ui` as its first
+    argument) so that unconsumed mock expectations are reported after the
+    command finishes, even if it aborts.
+    """
+    from sapling import extensions
+
+    def wrapped(orig, ui, *args, **kwargs):
+        try:
+            return orig(ui, *args, **kwargs)
+        finally:
+            server.report_unconsumed(ui)
+
+    extensions.wrapfunction(module, funcname, wrapped)
+
     def expect_get_repository_request(
         self, owner: str = OWNER, name: str = REPO_NAME
     ) -> "GetRepositoryRequest":

eden/scm/tests/test-doctest.py

@@ -32,6 +32,7 @@ def testmod(name, optionflags=0, testtarget=None):
 
 testmod("sapling.pathlog")
 testmod("sapling.ext.github.archive_commit")
+testmod("sapling.ext.github.github_gh_cli")
 testmod("sapling.ext.github.github_repo_util")
 testmod("sapling.ext.github.pr_parser")
 testmod("sapling.ext.github.pull_request_arg")