chore(pre-receive): add explanation to magic number.

Squash tests into parametrize
GitGuardian · Sep 30, 2021 · cd927ab · cd927ab
1 parent 4838736
commit cd927ab
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 62 deletions.
diff --git a/ggshield/utils.py b/ggshield/utils.py
@@ -28,6 +28,9 @@
 
 EMPTY_SHA = "0000000000000000000000000000000000000000"
 EMPTY_TREE = "4b825dc642cb6eb9a060e54bf8d69288fbee4904"
+
+# GitHub timeouts every pre-receive hook after 5s with an error.
+# We try and anticipate that so we can control the return code
 PRERECEIVE_TIMEOUT = 4.5
 
 

diff --git a/tests/test_prepush_cmd.py b/tests/test_prepush_cmd.py
@@ -1,5 +1,7 @@
+from typing import Dict, Optional
 from unittest.mock import ANY, Mock, patch
 
+import pytest
 from click.testing import CliRunner
 
 from ggshield.cmd import cli
@@ -42,26 +44,46 @@ def test_prepush_too_many(self, get_list_mock: Mock, cli_fs_runner: CliRunner):
     @patch("ggshield.hook_cmd.get_list_commit_SHA")
     @patch("ggshield.hook_cmd.scan_commit_range")
     @patch("ggshield.hook_cmd.check_git_dir")
-    def test_prepush_pre_commit_framework_new(
+    @pytest.mark.parametrize(
+        ["env", "input"],
+        [
+            pytest.param(
+                {"PRE_COMMIT_SOURCE": "a" * 40, "PRE_COMMIT_ORIGIN": "b" * 40},
+                None,
+                id="old env names",
+            ),
+            pytest.param(
+                {"PRE_COMMIT_FROM_REF": "a" * 40, "PRE_COMMIT_TO_REF": "b" * 40},
+                None,
+                id="new env names",
+            ),
+            pytest.param(
+                {},
+                f"main\n{'a'*40}\norigin/main\n{'b'*40}\n",
+                id="stdin input",
+            ),
+        ],
+    )
+    def test_prepush_pre_commit_framework(
         self,
         check_dir_mock: Mock,
         scan_commit_range_mock: Mock,
         get_list_mock: Mock,
         cli_fs_runner: CliRunner,
+        env: Dict,
+        input: Optional[str],
     ):
         """
-        GIVEN a prepush range with a 20 commits through the new env vars of the framework
+        GIVEN a prepush range with a 20 commits through the old env vars of the framework
         WHEN the command is run
         THEN it should pass onto scan and return 0
         """
         scan_commit_range_mock.return_value = 0
-        commit_list = ["a" for _ in range(20)]
+        commit_list = ["a"] * 20
         get_list_mock.return_value = commit_list
 
         result = cli_fs_runner.invoke(
-            cli,
-            ["-v", "scan", "pre-push"],
-            env={"PRE_COMMIT_FROM_REF": "a" * 40, "PRE_COMMIT_TO_REF": "b" * 40},
+            cli, ["-v", "scan", "pre-push"], env=env, input=input
         )
         get_list_mock.assert_called_once_with(
             "--max-count=51 " + "b" * 40 + "..." + "a" * 40
@@ -79,61 +101,6 @@ def test_prepush_pre_commit_framework_new(
             mode_header="pre_push",
             banlisted_detectors=set(),
         )
-        assert "Commits to scan: 20" in result.output
-        assert result.exit_code == 0
-
-    @patch("ggshield.hook_cmd.get_list_commit_SHA")
-    @patch("ggshield.hook_cmd.scan_commit_range")
-    @patch("ggshield.hook_cmd.check_git_dir")
-    def test_prepush_pre_commit_framework_old(
-        self,
-        check_dir_mock: Mock,
-        scan_commit_range_mock: Mock,
-        get_list_mock: Mock,
-        cli_fs_runner: CliRunner,
-    ):
-        """
-        GIVEN a prepush range with a 20 commits through the old env vars of the framework
-        WHEN the command is run
-        THEN it should pass onto scan and return 0
-        """
-        scan_commit_range_mock.return_value = 0
-        get_list_mock.return_value = ["a" for _ in range(20)]
-
-        result = cli_fs_runner.invoke(
-            cli,
-            ["-v", "scan", "pre-push"],
-            env={"PRE_COMMIT_SOURCE": "a" * 40, "PRE_COMMIT_ORIGIN": "b" * 40},
-        )
-        get_list_mock.assert_called_once_with(
-            "--max-count=51 " + "b" * 40 + "..." + "a" * 40
-        )
-        scan_commit_range_mock.assert_called_once()
-        assert "Commits to scan: 20" in result.output
-        assert result.exit_code == 0
-
-    @patch("ggshield.hook_cmd.get_list_commit_SHA")
-    @patch("ggshield.hook_cmd.scan_commit_range")
-    @patch("ggshield.hook_cmd.check_git_dir")
-    def test_prepush_stdin_input(
-        self,
-        check_dir_mock: Mock,
-        scan_commit_range_mock: Mock,
-        get_list_mock: Mock,
-        cli_fs_runner: CliRunner,
-    ):
-        """
-        GIVEN 20 commits through stdin input
-        WHEN the command is run
-        THEN it should pass onto scan and return 0
-        """
-        scan_commit_range_mock.return_value = 0
-        get_list_mock.return_value = ["a" for _ in range(20)]
-
-        result = cli_fs_runner.invoke(
-            cli, ["-v", "scan", "pre-push"], input="main\naaaa\norigin/main\nbbbb\n"
-        )
-        get_list_mock.assert_called_once_with("--max-count=51 bbbb" + "..." + "aaaa")
         scan_commit_range_mock.assert_called_once()
         assert "Commits to scan: 20" in result.output
         assert result.exit_code == 0

diff --git a/tests/test_prereceive_cmd.py b/tests/test_prereceive_cmd.py
@@ -106,7 +106,6 @@ def test_stdin_breakglass_2ndoption(
         WHEN the command is run
         THEN it should return 0
         """
-        scan_commit_range_mock.return_value = 0
         get_list_mock.return_value = ["a" for _ in range(20)]
 
         result = cli_fs_runner.invoke(