From 1fbd216dbc61bf1bdf07b5639576df71f095dc92 Mon Sep 17 00:00:00 2001 From: Jiewei Qian Date: Tue, 9 Jul 2024 23:32:32 +0000 Subject: [PATCH] metadata: expose validation result additional as a getter This permit downstream clients to retrieve the "source" text and do their own formatting instead of relying on the format coded in get_message(). Change-Id: Ia36cbd064ed0781bda76b09b064b97f6dc5e899e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5686730 Commit-Queue: Jiewei Qian Reviewed-by: Anne Redulla --- metadata/tests/validate_test.py | 41 +++++++++++++++++++++++++++++++++ metadata/validation_result.py | 34 ++++++++++++++------------- 2 files changed, 59 insertions(+), 16 deletions(-) diff --git a/metadata/tests/validate_test.py b/metadata/tests/validate_test.py index f878afbd9..c45ebe70d 100644 --- a/metadata/tests/validate_test.py +++ b/metadata/tests/validate_test.py @@ -16,6 +16,7 @@ sys.path.insert(0, _ROOT_DIR) import gclient_utils import metadata.validate +import metadata.validation_result # Common paths for tests. _SOURCE_FILE_DIR = os.path.join(_THIS_DIR, "data") @@ -141,5 +142,45 @@ class CheckFileTest(unittest.TestCase): self.assertEqual(len(warnings), 9) +class ValidationResultTest(unittest.TestCase): + """Tests ValidationResult handles strings correctly.""" + + def test_ordering(self): + ve = metadata.validation_result.ValidationError( + "abc", + ["message1", "message2"], + ) + + vw = metadata.validation_result.ValidationError( + "def", + ["message3", "message4"], + ) + + # Check errors preceeds warnings. + self.assertLess(ve, vw) + self.assertGreater(vw, ve) + self.assertEqual([ve, vw], list(sorted([vw, ve]))) + + def test_message_generation(self): + ve = metadata.validation_result.ValidationError( + "abc", + ["message1", "message2"], + ) + self.assertEqual( + ("Third party metadata issue: abc message1 message2 Check " + "//third_party/README.chromium.template for details."), + ve.get_message()) + self.assertEqual("abc message1 message2", + ve.get_message(prescript='', postscript='')) + + def test_getters(self): + ve = metadata.validation_result.ValidationError( + "abc", + ["message1", "message2"], + ) + self.assertEqual("abc", ve.get_reason()) + self.assertEqual(["message1", "message2"], ve.get_additional()) + + if __name__ == "__main__": unittest.main() diff --git a/metadata/validation_result.py b/metadata/validation_result.py index 4ffa219e4..a4adae6aa 100644 --- a/metadata/validation_result.py +++ b/metadata/validation_result.py @@ -4,7 +4,7 @@ # found in the LICENSE file. import textwrap -from typing import Dict, List, Optional +from typing import Dict, List, Optional, Tuple _CHROMIUM_METADATA_PRESCRIPT = "Third party metadata issue:" _CHROMIUM_METADATA_POSTSCRIPT = ("Check //third_party/README.chromium.template " @@ -26,42 +26,40 @@ class ValidationResult: """ self._reason = reason self._fatal = fatal - self._message = " ".join([reason] + additional) + self._additional = additional self._tags = {} def __str__(self) -> str: prefix = self.get_severity_prefix() - return f"{prefix} - {self._message}" + additional_text = ' '.join(self._additional) + return f"{prefix} - {self._reason} {additional_text}" def __repr__(self) -> str: return str(self) + def _comparision_key(self) -> Tuple: + return (not self._fatal, self._reason, self._additional) + # PEP 8 recommends implementing all 6 rich comparisons. # Here we make use of tuple comparison, and order based on the severity # (e.g. fatal comes before non-fatal), then the message. def __lt__(self, other) -> bool: - return (not self._fatal, self._message) < (not other._fatal, - other._message) + return self._comparision_key() < other._comparision_key() def __le__(self, other) -> bool: - return (not self._fatal, self._message) <= (not other._fatal, - other._message) + return self._comparision_key() <= other._comparision_key() def __gt__(self, other) -> bool: - return (not self._fatal, self._message) > (not other._fatal, - other._message) + return self._comparision_key() > other._comparision_key() def __ge__(self, other) -> bool: - return (not self._fatal, self._message) >= (not other._fatal, - other._message) + return self._comparision_key() >= other._comparision_key() def __eq__(self, other) -> bool: - return (not self._fatal, self._message) == (not other._fatal, - other._message) + return self._comparision_key() == other._comparision_key() def __ne__(self, other) -> bool: - return (not self._fatal, self._message) != (not other._fatal, - other._message) + return self._comparision_key() != other._comparision_key() def is_fatal(self) -> bool: return self._fatal @@ -83,11 +81,15 @@ class ValidationResult: def get_all_tags(self) -> Dict[str, str]: return dict(self._tags) + def get_additional(self) -> List[str]: + return self._additional + def get_message(self, prescript: str = _CHROMIUM_METADATA_PRESCRIPT, postscript: str = _CHROMIUM_METADATA_POSTSCRIPT, width: int = 0) -> str: - components = [prescript, self._message, postscript] + additional_text = ' '.join(self._additional) + components = [prescript, self._reason, additional_text, postscript] message = " ".join( [component for component in components if len(component) > 0])