From 813758a98d5680827a5a935b5d01bfc310751ffd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tina=20M=C3=BCller?= <cpan2@tinita.de>
Date: Sat, 18 May 2024 14:50:08 +0200
Subject: [PATCH] Restructure tests

- Rename _set_mock_data to _test_issue_reminder (before the name suggested
  that it was only setting mock data)
- Use named params. The `params` array was a bit hard to read, and the
  different length with the differing item inserted somewhere in the middle
  instead of the end made the code needlessly confusing. Also it contained
  unnecessary data
- Both test_automatic_priority_on_issue and
  test_issue_with_low_priority_never_change now assert the complete list of
  function calls
---
 tests/test_comments.py | 84 +++++++++++++++++++++---------------------
 1 file changed, 42 insertions(+), 42 deletions(-)

diff --git a/tests/test_comments.py b/tests/test_comments.py
index 443ca6f..3e81fef 100755
--- a/tests/test_comments.py
+++ b/tests/test_comments.py
@@ -110,55 +110,60 @@ def test_empty_issue(self):
 
     def test_automatic_priority_on_issue(self):
         test_params = [
-            # slo_from, slo_to, slo_id, slo_days
-            ["Immediate", "Urgent", 7, 2],
-            ["Urgent", "High", 6, 25],
-            ["High", "Normal", 5, 35],
-            ["Normal", "Low", 4, 700]]
+            # prio_from, prio_to, past_days, prio_id_to
+            ["Immediate", "Urgent", 2, 6],
+            ["Urgent", "High", 25, 5],
+            ["High", "Normal", 35, 4],
+            ["Normal", "Low", 700, 3]]
         expected_str = "Reducing priority from {} to next lower {} for 1000"
         for params in test_params:
+            prio_from = params[0]
+            prio_to = params[1]
+            prio_id_to = params[3]
             with self.subTest(params):
-                self._set_mock_data(params)
+                self._test_issue_reminder(prio_from=params[0], past_days=params[2])
                 out, err = self.capsys.readouterr()
-                assert re.search(expected_str.format(params[0],
-                                                     params[1]), out)
-        calls = [
-            call(
-                "GET",
-                "https://example.com/wiki/1000.json?include=journals",
-            ),
-            call(
-                "PUT",
-                "https://example.com/wiki/1000.json",
-                {
-                    "issue": {
-                        "priority_id": 3,
-                        "notes": "This ticket was set to **Normal** priority but was not updated [within the SLO period](https://example.com/issues). The ticket will be set to the next lower priority **Low**."
-                    }
-                },
-            ),
-        ]
-        backlogger.json_rest.assert_has_calls(calls)
+                assert re.search(expected_str.format(prio_from,
+                                                     prio_to), out)
+            calls = [
+                call(
+                    "GET",
+                    "https://example.com/wiki/1000.json?include=journals",
+                ),
+                call(
+                    "PUT",
+                    "https://example.com/wiki/1000.json",
+                    {
+                        "issue": {
+                            "priority_id": prio_id_to,
+                            "notes": "This ticket was set to **{}** priority but was not updated [within the SLO period](https://example.com/issues). The ticket will be set to the next lower priority **{}**.".format(prio_from, prio_to)
+                        }
+                    },
+                ),
+            ]
+            backlogger.json_rest.assert_has_calls(calls)
 
 
     def test_issue_with_low_priority_never_change(self):
         test_params = [
-            # slo_from, slo_id, slo_days
-            ["Low", 3, 2],
-            ["Low", 3, 1000]]
+            # prio_from, past_days
+            ["Low", 2],
+            ["Low", 1000]]
         for params in test_params:
             with self.subTest(params):
-                self._set_mock_data(params)
+                self._test_issue_reminder(prio_from=params[0], past_days=params[1])
                 out, err = self.capsys.readouterr()
                 assert re.search("Skipping priority update for 1000", out)
+            calls = [
+                call(
+                    "GET",
+                    "https://example.com/wiki/1000.json?include=journals",
+                ),
+            ]
+            backlogger.json_rest.assert_has_calls(calls)
 
-    def _set_mock_data(self, params):
-        if len(params) > 3:
-            past_days = params[3]
-            id = params[1]
-        else:
-            past_days = params[2]
-            id = params[1]
+
+    def _test_issue_reminder(self, prio_from, past_days):
         data = {"url": "https://example.com/issues", "web": "https://example.com/wiki",
                 "api": "https://example.com/issues.json",
                 "reminder-comment-on-issues": True}
@@ -166,7 +171,6 @@ def _set_mock_data(self, params):
         rest = {
             "issue": {
                 "id": 1000,
-                "priority": { "id": id, "name": params[0] },
                 "journals": [
                     { "id": 1, "notes": "" },
                     { "id": 2, "notes": None },
@@ -182,10 +186,6 @@ def _set_mock_data(self, params):
         backlogger.json_rest = MagicMock(return_value=rest)
         backlogger.issue_reminder(
             {"query": "query_id=123&c%5B%5D=updated_on"},
-            {"priority": {"name": params[0]}, "id": 1000},
+            {"priority": {"name": prio_from}, "id": 1000},
             {'has_repeat_reminder': datetime.min, 'last_reminder': False}
         )
-        backlogger.json_rest.assert_any_call(
-            "GET",
-            "https://example.com/wiki/1000.json?include=journals",
-        )