using regex for slugs, reconstruct code to follow cli's architecture

Author: dana-yaish

codecov_cli/commands/base_picking.py

@@ -5,8 +5,8 @@
 import click
 
 from codecov_cli.fallbacks import CodecovOption, FallbackFieldEnum
-from codecov_cli.helpers.encoder import slug_is_invalid
-from codecov_cli.helpers.request import send_put_request
+from codecov_cli.helpers.encoder import slug_without_subgroups_is_invalid
+from codecov_cli.services.commit.base_picking import base_picking_logic
 
 logger = logging.getLogger("codecovcli")
 
@@ -66,30 +66,15 @@ def pr_base_picking(
         ),
     )
 
-    if slug_is_invalid(slug):
+    if slug_without_subgroups_is_invalid(slug):
         logger.error(
             "Slug is invalid. Slug should be in the form of owner_username/repo_name"
         )
         return
 
-    data = {
-        "user_provided_base_sha": base_sha,
-    }
-    headers = {"Authorization": f"token {token.hex}"}
-    url = f"https://api.codecov.io/api/v1/{service}/{slug}/pulls/{pr}"
-    sending_result = send_put_request(url=url, data=data, headers=headers)
-
-    if sending_result.warnings:
-        number_warnings = len(sending_result.warnings)
-        pluralization = "s" if number_warnings > 1 else ""
+    res = base_picking_logic(base_sha, pr, slug, token, service)
+    if not res.error:
         logger.info(
-            f"Base picking process had {number_warnings} warning{pluralization}",
+            "Base picking finished successfully",
+            extra=dict(extra_log_attributes=dict(response=res.text)),
         )
-        for ind, w in enumerate(sending_result.warnings):
-            logger.warning(f"Warning {ind + 1}: {w}")
-    if sending_result.error is not None:
-        logger.error(f"Base picking failed: {sending_result.error.description}")
-        return
-
-    logger.info("Base picking finished successfully")
-    logger.info(sending_result.text)

codecov_cli/helpers/encoder.py

@@ -1,13 +1,29 @@
+import re
+
+slug_without_subgroups_regex = re.compile(r"[^/\s]+\/[^/\s]+$")
+slug_with_subgroups_regex = re.compile(r"[^/\s]+(\/[^/\s]+)+$")
+
+
 def encode_slug(slug: str):
-    if "/" not in slug:
+    if slug_with_subgroups_is_invalid(slug):
         raise ValueError("The provided slug is invalid")
     owner, repo = slug.rsplit("/", 1)
     encoded_owner = ":::".join(owner.split("/"))
     encoded_slug = "::::".join([encoded_owner, repo])
     return encoded_slug
 
 
-def slug_is_invalid(slug: str):
-    if "/" not in slug or slug.count("/") > 1:
-        return True
-    return False
+def slug_without_subgroups_is_invalid(slug: str):
+    """
+    Checks if slug is in the form of owner/repo
+    Returns True if it's invalid, otherwise return False
+    """
+    return not slug or not slug_without_subgroups_regex.match(slug)
+
+
+def slug_with_subgroups_is_invalid(slug: str):
+    """
+    Checks if slug is in the form of owner/repo or owner/subgroup/repo
+    Returns True if it's invalid, otherwise return False
+    """
+    return not slug or not slug_with_subgroups_regex.match(slug)

codecov_cli/services/commit/base_picking.py

@@ -0,0 +1,17 @@
+import logging
+
+from codecov_cli.helpers.request import log_warnings_and_errors_if_any, send_put_request
+
+logger = logging.getLogger("codecovcli")
+
+
+def base_picking_logic(base_sha, pr, slug, token, service):
+    data = {
+        "user_provided_base_sha": base_sha,
+    }
+    headers = {"Authorization": f"token {token.hex}"}
+    url = f"https://api.codecov.io/api/v1/{service}/{slug}/pulls/{pr}"
+    sending_result = send_put_request(url=url, data=data, headers=headers)
+
+    log_warnings_and_errors_if_any(sending_result, "Base picking")
+    return sending_result

tests/helpers/test_encoder.py

@@ -1,31 +1,55 @@
 import pytest
 
-from codecov_cli.helpers.encoder import encode_slug, slug_is_invalid
-
-
-def test_invalid_slug():
-    slug = "invalid-slug"
+from codecov_cli.helpers.encoder import encode_slug, slug_without_subgroups_is_invalid
+
+
+@pytest.mark.parametrize(
+    "slug",
+    [
+        ("invalid_slug"),
+        (""),
+        ("/"),
+        ("//"),
+        ("///"),
+        ("random string"),
+        (None),
+    ],
+)
+def test_encode_invalid_slug(slug):
     with pytest.raises(ValueError) as ex:
         encode_slug(slug)
 
 
-def test_encode_slug():
-    slug = "owner/repo"
-    encoded_slug = encode_slug(slug)
-    assert encoded_slug == "owner::::repo"
-
-
-def test_encode_owner_with_subgroups_slug():
-    slug = "owner/subgroup/repo"
-    encoded_slug = encode_slug(slug)
-    assert encoded_slug == "owner:::subgroup::::repo"
-
-
-def test_invalid_slug2():
-    slug = "invalid_slug"
-    assert slug_is_invalid(slug)
+@pytest.mark.parametrize(
+    "slug, encoded_slug",
+    [
+        ("owner/repo", "owner::::repo"),
+        ("owner/subgroup/repo", "owner:::subgroup::::repo"),
+    ],
+)
+def test_encode_valid_slug(slug, encoded_slug):
+    expected_encoded_slug = encode_slug(slug)
+    assert expected_encoded_slug == encoded_slug
+
+
+@pytest.mark.parametrize(
+    "slug",
+    [
+        ("invalid_slug"),
+        (""),
+        ("/"),
+        ("//"),
+        ("///"),
+        ("random string"),
+        ("owner/subgroup/repo"),
+        ("owner//repo"),
+        (None),
+    ],
+)
+def test_invalid_slug(slug):
+    assert slug_without_subgroups_is_invalid(slug)
 
 
 def test_valid_slug():
     slug = "owner/repo"
-    assert not slug_is_invalid(slug)
+    assert not slug_without_subgroups_is_invalid(slug)

tests/test_base_picking.py

@@ -4,12 +4,12 @@
 
 from codecov_cli.commands.base_picking import pr_base_picking
 from codecov_cli.main import cli
-from codecov_cli.types import RequestError, RequestResult
+from codecov_cli.types import RequestError, RequestResult, RequestResultWarning
 
 
 def test_base_picking_command(mocker):
     mocked_response = mocker.patch(
-        "codecov_cli.commands.base_picking.send_put_request",
+        "codecov_cli.services.commit.base_picking.send_put_request",
         return_value=RequestResult(status_code=200, error=None, warnings=[], text=""),
     )
     token = uuid.uuid4()
@@ -30,10 +30,6 @@ def test_base_picking_command(mocker):
         ],
     )
     assert result.exit_code == 0
-    assert (
-        f'debug: Starting base picking process --- {{"pr": "11", "slug": "owner/repo", "token": "{token}", "service": "github"}}'
-        in result.output
-    )
     assert "info: Base picking finished successfully" in result.output
     mocked_response.assert_called_once()
 
@@ -65,9 +61,12 @@ def test_base_picking_command_slug_invalid(mocker):
 
 def test_base_picking_command_warnings(mocker):
     mocked_response = mocker.patch(
-        "codecov_cli.commands.base_picking.send_put_request",
+        "codecov_cli.services.commit.base_picking.send_put_request",
         return_value=RequestResult(
-            status_code=200, error=None, warnings=["some random warning"], text=""
+            error=None,
+            warnings=[RequestResultWarning(message="some random warning")],
+            status_code=200,
+            text="",
         ),
     )
     token = uuid.uuid4()
@@ -88,10 +87,6 @@ def test_base_picking_command_warnings(mocker):
         ],
     )
     assert result.exit_code == 0
-    assert (
-        f'debug: Starting base picking process --- {{"pr": "11", "slug": "owner/repo", "token": "{token}", "service": "github"}}'
-        in result.output
-    )
     assert "info: Base picking process had 1 warning" in result.output
     assert "Warning 1: some random warning" in result.output
     assert "info: Base picking finished successfully" in result.output
@@ -100,7 +95,7 @@ def test_base_picking_command_warnings(mocker):
 
 def test_base_picking_command_error(mocker):
     mocked_response = mocker.patch(
-        "codecov_cli.commands.base_picking.send_put_request",
+        "codecov_cli.services.commit.base_picking.send_put_request",
         return_value=RequestResult(
             status_code=401,
             error=RequestError(
@@ -132,9 +127,5 @@ def test_base_picking_command_error(mocker):
     mocked_response.assert_called_once()
     print(result.output)
     assert result.exit_code == 0
-    assert (
-        f'debug: Starting base picking process --- {{"pr": "11", "slug": "owner/repo", "token": "{token}", "service": "github"}}'
-        in result.output
-    )
     assert "error: Base picking failed: Unauthorized" in result.output
     assert "info: Base picking finished successfully" not in result.output