From 7c0aa1bb6768f87955d4cc818aa878bd88701a87 Mon Sep 17 00:00:00 2001 From: bruntib Date: Fri, 8 Dec 2023 19:18:36 +0100 Subject: [PATCH] [test] Enable review status config tests. --- ...config.py => test_review_status_config.py} | 48 ++++++++++++------- codechecker_common/review_status_handler.py | 10 ++++ 2 files changed, 41 insertions(+), 17 deletions(-) rename analyzer/tests/unit/{review_status_config.py => test_review_status_config.py} (81%) diff --git a/analyzer/tests/unit/review_status_config.py b/analyzer/tests/unit/test_review_status_config.py similarity index 81% rename from analyzer/tests/unit/review_status_config.py rename to analyzer/tests/unit/test_review_status_config.py index a722f3e0a9..c276c644a4 100644 --- a/analyzer/tests/unit/review_status_config.py +++ b/analyzer/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 index e05998e737..1c46a8f3d9 100644 --- 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(