Merge pull request #4124 from bruntib/review_status_config_test_fix

[test] Enable review status config tests.
Ericsson · Dec 8, 2023 · bea15d4 · bea15d4
2 parents d6164d7 + 7c0aa1b
commit bea15d4
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 17 deletions.
diff --git a/analyzer/tests/unit/review_status_config.py → ...r/tests/unit/test_review_status_config.py b/analyzer/tests/unit/review_status_config.py → ...r/tests/unit/test_review_status_config.py
@@ -50,7 +50,8 @@ def __put_in_review_status_cfg_file(self, file_contents: str) -> str:
     def test_empty_file(self):
         cfg = ""
         rscfg_file = self.__put_in_review_status_cfg_file(cfg)
-        with self.assertRaisesRegex(ValueError, "$version"):
+        with self.assertRaisesRegex(
+                ValueError, "should represent a dictionary."):
             self.rshandler.set_review_status_config(rscfg_file)
 
     def test_no_version(self):
@@ -64,7 +65,8 @@ def test_no_version(self):
         """
 
         rscfg_file = self.__put_in_review_status_cfg_file(cfg)
-        with self.assertRaisesRegex(ValueError, "$version"):
+        with self.assertRaisesRegex(
+                ValueError, r"must contain the key '\$version'"):
             self.rshandler.set_review_status_config(rscfg_file)
 
     def test_empty_action_list(self):
@@ -77,7 +79,8 @@ def test_empty_action_list(self):
         """
 
         rscfg_file = self.__put_in_review_status_cfg_file(cfg)
-        with self.assertRaisesRegex(ValueError, "TODO"):
+        with self.assertRaisesRegex(
+                ValueError, "'actions' must have at least one element"):
             self.rshandler.set_review_status_config(rscfg_file)
 
     def test_empty_filter_list(self):
@@ -91,7 +94,8 @@ def test_empty_filter_list(self):
         """
 
         rscfg_file = self.__put_in_review_status_cfg_file(cfg)
-        with self.assertRaisesRegex(ValueError, "TODO"):
+        with self.assertRaisesRegex(
+                ValueError, "'filters' must have at least one element."):
             self.rshandler.set_review_status_config(rscfg_file)
 
     def test_empty_filter_and_action_list(self):
@@ -103,7 +107,8 @@ def test_empty_filter_and_action_list(self):
         """
 
         rscfg_file = self.__put_in_review_status_cfg_file(cfg)
-        with self.assertRaisesRegex(ValueError, "TODO"):
+        with self.assertRaisesRegex(
+                ValueError, "'filters' must have at least one element."):
             self.rshandler.set_review_status_config(rscfg_file)
 
     def test_empty_rules_list(self):
@@ -113,7 +118,9 @@ def test_empty_rules_list(self):
         """
 
         rscfg_file = self.__put_in_review_status_cfg_file(cfg)
-        with self.assertRaisesRegex(ValueError, "'rules'"):
+        with self.assertRaisesRegex(
+                ValueError,
+                "should contain the key 'rules' with a non-empty list of"):
             self.rshandler.set_review_status_config(rscfg_file)
 
     def test_correct(self):
@@ -142,8 +149,8 @@ def test_invalid_review_status(self):
         """
 
         rscfg_file = self.__put_in_review_status_cfg_file(cfg)
-        with self.assertRaisesRegex(ValueError,
-                                    "Invalid review status field: "):
+        with self.assertRaisesRegex(
+                ValueError, "Invalid review status field: "):
             self.rshandler.set_review_status_config(rscfg_file)
 
     def test_invalid_action(self):
@@ -158,8 +165,8 @@ def test_invalid_action(self):
         """
 
         rscfg_file = self.__put_in_review_status_cfg_file(cfg)
-        with self.assertRaisesRegex(ValueError,
-                                    "'bake' is not allowed"):
+        with self.assertRaisesRegex(
+                ValueError, "'bake' is not allowed"):
             self.rshandler.set_review_status_config(rscfg_file)
 
     def test_no_review_status(self):
@@ -173,8 +180,8 @@ def test_no_review_status(self):
         """
 
         rscfg_file = self.__put_in_review_status_cfg_file(cfg)
-        with self.assertRaisesRegex(ValueError,
-                                    "review_status' .* required"):
+        with self.assertRaisesRegex(
+                ValueError, "review_status' .* required"):
             self.rshandler.set_review_status_config(rscfg_file)
 
     def test_invalid_filter(self):
@@ -189,8 +196,8 @@ def test_invalid_filter(self):
         """
 
         rscfg_file = self.__put_in_review_status_cfg_file(cfg)
-        with self.assertRaisesRegex(ValueError,
-                                    "'favourite_color' is not allowed"):
+        with self.assertRaisesRegex(
+                ValueError, "'favourite_color' is not allowed"):
             self.rshandler.set_review_status_config(rscfg_file)
 
     def test_no_actions(self):
@@ -218,7 +225,9 @@ def test_no_filters(self):
         with self.assertRaisesRegex(ValueError, "'filters' and 'actions'"):
             self.rshandler.set_review_status_config(rscfg_file)
 
-    def test_no_reason(self):
+    # TODO: The "reason" field is not required. It is also not required for
+    # review statuses with source code comment.
+    def no_reason(self):
         cfg = """
 $version: 1
 rules:
@@ -252,10 +261,15 @@ def test_no_rules2(self):
         """
 
         rscfg_file = self.__put_in_review_status_cfg_file(cfg)
-        with self.assertRaisesRegex(ValueError, "'rules'"):
+        with self.assertRaisesRegex(
+                ValueError,
+                "should contain the key 'rules' with a non-empty list of"):
             self.rshandler.set_review_status_config(rscfg_file)
 
-    def test_multiple_checker_keys(self):
+    # TODO: I'm not sure if we can check this. The yaml parser accepts this and
+    # later there is no opportunity to check double keys because we have a
+    # valid Python object only.
+    def multiple_checker_keys(self):
         cfg = """
 $version: 1
 rules:

diff --git a/codechecker_common/review_status_handler.py b/codechecker_common/review_status_handler.py
@@ -104,6 +104,16 @@ def __check_format_version_1(self):
                     f"'filters' and 'actions':\n"
                     f"{yaml.dump(rule)}")
 
+            if rule['filters'] is None:
+                raise ValueError(
+                    f"Format error in {self.__review_status_yaml}: 'filters' "
+                    f"must have at least one element.")
+
+            if rule['actions'] is None:
+                raise ValueError(
+                    f"Format error in {self.__review_status_yaml}: 'actions' "
+                    f"must have at least one element.")
+
             for field in rule['filters']:
                 if field not in ReviewStatusHandler.ALLOWED_FILTERS:
                     raise ValueError(