From 2c68b62640c29ad538aab641d162b5fd3daf9bdf Mon Sep 17 00:00:00 2001 From: JWM Date: Thu, 17 Oct 2024 15:21:21 +0200 Subject: [PATCH 1/7] Use cq_findings as property; Extend when iterating over checkers No need to share objects with sub-checkers --- src/mlx/warnings/polyspace_checker.py | 8 +++++++- src/mlx/warnings/regex_checker.py | 8 +++++++- src/mlx/warnings/warnings_checker.py | 7 ++++++- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/mlx/warnings/polyspace_checker.py b/src/mlx/warnings/polyspace_checker.py index e3aa317a..bc152f62 100644 --- a/src/mlx/warnings/polyspace_checker.py +++ b/src/mlx/warnings/polyspace_checker.py @@ -17,6 +17,13 @@ def __init__(self, verbose): super().__init__(verbose) self._cq_description_template = Template('Polyspace: $check') + @property + def cq_findings(self): + ''' List[dict]: list of code quality findings''' + for checker in self.checkers: + self._cq_findings.extend(checker.cq_findings) + return self._cq_findings + @property def counted_warnings(self): ''' List: list of counted warnings (str) ''' @@ -146,7 +153,6 @@ def parse_config(self, config): check_value = value.lower() checker = PolyspaceFamilyChecker(family_value, column_name, check_value, verbose=self.verbose) checker.parse_config(check) - checker.cq_findings = self.cq_findings # share object with sub-checkers self.checkers.append(checker) if not (column_name and check_value): raise ValueError( diff --git a/src/mlx/warnings/regex_checker.py b/src/mlx/warnings/regex_checker.py index ecdad437..5db8a53f 100644 --- a/src/mlx/warnings/regex_checker.py +++ b/src/mlx/warnings/regex_checker.py @@ -117,6 +117,13 @@ def counted_warnings(self): all_counted_warnings.extend(checker.counted_warnings) return all_counted_warnings + @property + def cq_findings(self): + ''' List[dict]: list of code quality findings''' + for checker in self.checkers.values(): + self._cq_findings.extend(checker.cq_findings) + return self._cq_findings + @property def cq_description_template(self): ''' Template: string.Template instance based on the configured template string ''' @@ -193,7 +200,6 @@ def parse_config(self, config): checker.maximum = int(maximum) if minimum := checker_config.get("min", 0): checker.minimum = int(minimum) - checker.cq_findings = self.cq_findings # share object with sub-checkers self.checkers[classification_key] = checker else: print(f"WARNING: Unrecognized classification {classification!r}") diff --git a/src/mlx/warnings/warnings_checker.py b/src/mlx/warnings/warnings_checker.py index 3562cd47..e13e19ab 100644 --- a/src/mlx/warnings/warnings_checker.py +++ b/src/mlx/warnings/warnings_checker.py @@ -42,13 +42,18 @@ def __init__(self, verbose=False): self._minimum = 0 self._maximum = 0 self._counted_warnings = [] - self.cq_findings = [] + self._cq_findings = [] self.cq_enabled = False self.cq_default_path = '.gitlab-ci.yml' self._cq_description_template = Template('$description') self.exclude_patterns = [] self.include_patterns = [] + @property + def cq_findings(self): + ''' List[dict]: list of code quality findings''' + return self._cq_findings + @property def counted_warnings(self): ''' List: list of counted warnings (str) ''' From c35368499f9f4c4c5c368672fb363d749e7624a9 Mon Sep 17 00:00:00 2001 From: JWM Date: Thu, 17 Oct 2024 15:21:45 +0200 Subject: [PATCH 2/7] Update docstring --- src/mlx/warnings/polyspace_checker.py | 2 +- src/mlx/warnings/regex_checker.py | 2 +- src/mlx/warnings/robot_checker.py | 2 +- src/mlx/warnings/warnings_checker.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/mlx/warnings/polyspace_checker.py b/src/mlx/warnings/polyspace_checker.py index bc152f62..07ed21d1 100644 --- a/src/mlx/warnings/polyspace_checker.py +++ b/src/mlx/warnings/polyspace_checker.py @@ -26,7 +26,7 @@ def cq_findings(self): @property def counted_warnings(self): - ''' List: list of counted warnings (str) ''' + '''List[str]: list of counted warnings''' all_counted_warnings = [] for checker in self.checkers: all_counted_warnings.extend(checker.counted_warnings) diff --git a/src/mlx/warnings/regex_checker.py b/src/mlx/warnings/regex_checker.py index 5db8a53f..dc6075c8 100644 --- a/src/mlx/warnings/regex_checker.py +++ b/src/mlx/warnings/regex_checker.py @@ -111,7 +111,7 @@ def __init__(self, verbose=False): @property def counted_warnings(self): - ''' List: list of counted warnings (str) ''' + ''' List[str]: list of counted warnings''' all_counted_warnings = [] for checker in self.checkers.values(): all_counted_warnings.extend(checker.counted_warnings) diff --git a/src/mlx/warnings/robot_checker.py b/src/mlx/warnings/robot_checker.py index 5a1e7c04..809b037b 100644 --- a/src/mlx/warnings/robot_checker.py +++ b/src/mlx/warnings/robot_checker.py @@ -12,7 +12,7 @@ class RobotChecker(WarningsChecker): @property def counted_warnings(self): - ''' List: list of counted warnings (str) ''' + '''List[str]: list of counted warnings''' all_counted_warnings = [] for checker in self.checkers: all_counted_warnings.extend(checker.counted_warnings) diff --git a/src/mlx/warnings/warnings_checker.py b/src/mlx/warnings/warnings_checker.py index e13e19ab..d2d9ed2c 100644 --- a/src/mlx/warnings/warnings_checker.py +++ b/src/mlx/warnings/warnings_checker.py @@ -56,7 +56,7 @@ def cq_findings(self): @property def counted_warnings(self): - ''' List: list of counted warnings (str) ''' + ''' List[str]: list of counted warnings''' return self._counted_warnings @property From 710d93f3f7e417939f8377460eeb80b752df6704 Mon Sep 17 00:00:00 2001 From: JWM Date: Thu, 17 Oct 2024 15:23:05 +0200 Subject: [PATCH 3/7] Add coverity tests with config file and code quality report --- tests/test_coverity.py | 30 +++++- tests/test_in/config_example_coverity.yml | 24 +++++ tests/test_in/coverity_cq.json | 114 ++++++++++++++++++++++ tests/test_in/defects.txt | 8 ++ 4 files changed, 175 insertions(+), 1 deletion(-) create mode 100644 tests/test_in/config_example_coverity.yml create mode 100644 tests/test_in/coverity_cq.json create mode 100644 tests/test_in/defects.txt diff --git a/tests/test_coverity.py b/tests/test_coverity.py index 03dcd608..d562b74e 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -1,10 +1,14 @@ from io import StringIO from unittest import TestCase +from pathlib import Path +import filecmp from unittest.mock import patch -from mlx.warnings import WarningsPlugin +from mlx.warnings import WarningsPlugin, warnings_wrapper +TEST_IN_DIR = Path(__file__).parent / 'test_in' +TEST_OUT_DIR = Path(__file__).parent / 'test_out' class TestCoverityWarnings(TestCase): def setUp(self): @@ -47,3 +51,27 @@ def test_single_warning_real_output(self): self.warnings.check(dut3) self.assertEqual(self.warnings.return_count(), 1) self.assertIn(dut2, fake_out.getvalue()) + + def test_code_quality_without_config(self): + filename = 'coverity_cq.json' + out_file = str(TEST_OUT_DIR / filename) + ref_file = str(TEST_IN_DIR / filename) + retval = warnings_wrapper([ + '--coverity', + '--code-quality', out_file, + str(TEST_IN_DIR / 'defects.txt'), + ]) + self.assertEqual(8, retval) + self.assertTrue(filecmp.cmp(out_file, ref_file)) + + def test_code_quality_with_config(self): + filename = 'coverity_cq.json' + out_file = str(TEST_OUT_DIR / filename) + ref_file = str(TEST_IN_DIR / filename) + retval = warnings_wrapper([ + '--code-quality', out_file, + '--config', str(TEST_IN_DIR / 'config_example_coverity.yml'), + str(TEST_IN_DIR / 'defects.txt'), + ]) + self.assertEqual(3, retval) + self.assertTrue(filecmp.cmp(out_file, ref_file)) diff --git a/tests/test_in/config_example_coverity.yml b/tests/test_in/config_example_coverity.yml new file mode 100644 index 00000000..52f1eba5 --- /dev/null +++ b/tests/test_in/config_example_coverity.yml @@ -0,0 +1,24 @@ +coverity: + enabled: true + intentional: + min: 0 + max: -1 + bug: + max: 0 + pending: + min: 0 + max: 0 + false_positive: + max: -1 +sphinx: + enabled: false +doxygen: + enabled: false +junit: + enabled: false +xmlrunner: + enabled: false +robot: + enabled: false +polyspace: + enabled: false diff --git a/tests/test_in/coverity_cq.json b/tests/test_in/coverity_cq.json new file mode 100644 index 00000000..e2ef5af5 --- /dev/null +++ b/tests/test_in/coverity_cq.json @@ -0,0 +1,114 @@ +[ + { + "severity": "info", + "location": { + "path": "some/path/dummy_int.h", + "positions": { + "begin": { + "line": 150, + "column": 16 + } + } + }, + "description": "Coverity: MISRA C-2012 Standard C Environment (MISRA C-2012 Rule 1.2, Advisory)", + "fingerprint": "26ce9a2e6eb1a2186f10995b6261e7ef" + }, + { + "severity": "info", + "location": { + "path": "some/path/dummy_int.h", + "positions": { + "begin": { + "line": 164, + "column": 16 + } + } + }, + "description": "Coverity: MISRA C-2012 Standard C Environment (MISRA C-2012 Rule 1.2, Advisory)", + "fingerprint": "a007582ec0592332f1db4626436aef22" + }, + { + "severity": "info", + "location": { + "path": "some/path/dummy_int.h", + "positions": { + "begin": { + "line": 34, + "column": 12 + } + } + }, + "description": "Coverity: MISRA C-2012 Standard C Environment (MISRA C-2012 Rule 1.2, Advisory)", + "fingerprint": "483e97b9e911d1091876456f0e05136a" + }, + { + "severity": "info", + "location": { + "path": "some/path/dummy_fp.c", + "positions": { + "begin": { + "line": 367, + "column": 13 + } + } + }, + "description": "Coverity: Out-of-bounds write (OVERRUN)", + "fingerprint": "d95a012dc6fca902fe73142c81846152" + }, + { + "severity": "info", + "location": { + "path": "some/path/dummy_fp.c", + "positions": { + "begin": { + "line": 367, + "column": 13 + } + } + }, + "description": "Coverity: MISRA C-2012 Pointers and Arrays (MISRA C-2012 Rule 18.1, Required)", + "fingerprint": "40317d6bdf17c682f9fbc356209da168" + }, + { + "severity": "major", + "location": { + "path": "some/path/dummy_uncl.h", + "positions": { + "begin": { + "line": 194, + "column": 14 + } + } + }, + "description": "Coverity: MISRA C-2012 Declarations and Definitions (MISRA C-2012 Rule 8.5, Required)", + "fingerprint": "48e27cc7f6eef3c6e4f305a4613491ff" + }, + { + "severity": "major", + "location": { + "path": "some/path/dummy_uncl.c", + "positions": { + "begin": { + "line": 1404, + "column": 14 + } + } + }, + "description": "Coverity: MISRA C-2012 Declarations and Definitions (MISRA C-2012 Rule 8.6, Required)", + "fingerprint": "580c5e4df217d18403c7e53b9b1d21a7" + }, + { + "severity": "major", + "location": { + "path": "some/path/dummy_uncl.c", + "positions": { + "begin": { + "line": 923, + "column": 13 + } + } + }, + "description": "Coverity: MISRA C-2012 Identifiers (MISRA C-2012 Rule 5.8, Required)", + "fingerprint": "199398cb40462913b6da4b7ab00ca19d" + } +] diff --git a/tests/test_in/defects.txt b/tests/test_in/defects.txt new file mode 100644 index 00000000..4f0ab5ad --- /dev/null +++ b/tests/test_in/defects.txt @@ -0,0 +1,8 @@ +some/path/dummy_int.h:150:16: CID 417642 (#1 of 1): MISRA C-2012 Standard C Environment (MISRA C-2012 Rule 1.2, Advisory): Intentional, Minor, Ignore, owner is Unassigned, defect only exists locally. +some/path/dummy_int.h:164:16: CID 417639 (#1 of 1): MISRA C-2012 Standard C Environment (MISRA C-2012 Rule 1.2, Advisory): Intentional, Minor, Ignore, owner is Unassigned, defect only exists locally. +some/path/dummy_int.h:34:12: CID 264736 (#1 of 1): MISRA C-2012 Standard C Environment (MISRA C-2012 Rule 1.2, Advisory): Intentional, Minor, Ignore, owner is Unassigned, defect only exists locally. +some/path/dummy_fp.c:367:13: CID 423570 (#1 of 1): Out-of-bounds write (OVERRUN): False Positive, Minor, Ignore, owner is sfo, defect only exists locally. +some/path/dummy_fp.c:367:13: CID 423568 (#1 of 1): MISRA C-2012 Pointers and Arrays (MISRA C-2012 Rule 18.1, Required): False Positive, Minor, Ignore, owner is sfo, defect only exists locally. +some/path/dummy_uncl.h:194:14: CID 431350 (#1 of 1): MISRA C-2012 Declarations and Definitions (MISRA C-2012 Rule 8.5, Required): Unclassified, Unspecified, Undecided, owner is Unassigned, defect only exists locally. +some/path/dummy_uncl.c:1404:14: CID 431349 (#1 of 1): MISRA C-2012 Declarations and Definitions (MISRA C-2012 Rule 8.6, Required): Unclassified, Unspecified, Undecided, owner is Unassigned, defect only exists locally. +some/path/dummy_uncl.c:923:13: CID 431348 (#1 of 1): MISRA C-2012 Identifiers (MISRA C-2012 Rule 5.8, Required): Unclassified, Unspecified, Undecided, owner is Unassigned, defect only exists locally. From bddd397c5f84e8cd135d845d55a769d8727cfee0 Mon Sep 17 00:00:00 2001 From: JWM Date: Thu, 17 Oct 2024 15:23:59 +0200 Subject: [PATCH 4/7] Fix integration tests due to bug in code quality report of Coverity --- tests/test_in/code_quality.json | 28 ++++++++++++++++++++++++++ tests/test_in/code_quality_format.json | 28 ++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/tests/test_in/code_quality.json b/tests/test_in/code_quality.json index 3adac70c..c90fa4c8 100644 --- a/tests/test_in/code_quality.json +++ b/tests/test_in/code_quality.json @@ -75,5 +75,33 @@ }, "description": "test_some_error_test (something.anything.somewhere)'", "fingerprint": "cd09ae46ee5361570fd59de78b454e11" + }, + { + "severity": "major", + "location": { + "path": "src/somefile.c", + "positions": { + "begin": { + "line": 82, + "column": 1 + } + } + }, + "description": "Coverity: Coding standard violation (MISRA C-2012 Rule 10.1)", + "fingerprint": "2cfae17932f3385c6c745d75606871c7" + }, + { + "severity": "major", + "location": { + "path": "src/somefile.c", + "positions": { + "begin": { + "line": 82, + "column": 1 + } + } + }, + "description": "Coverity: Coding standard violation (MISRA C-2012 Rule 10.1)", + "fingerprint": "53754fd4b92326dc35bee26e774c9df3" } ] diff --git a/tests/test_in/code_quality_format.json b/tests/test_in/code_quality_format.json index 89ca3c02..c64cb880 100644 --- a/tests/test_in/code_quality_format.json +++ b/tests/test_in/code_quality_format.json @@ -75,5 +75,33 @@ }, "description": "test_some_error_test (something.anything.somewhere)'", "fingerprint": "cd09ae46ee5361570fd59de78b454e11" + }, + { + "severity": "major", + "location": { + "path": "src/somefile.c", + "positions": { + "begin": { + "line": 82, + "column": 1 + } + } + }, + "description": "Coverity: Coding standard violation (MISRA C-2012 Rule 10.1)", + "fingerprint": "2cfae17932f3385c6c745d75606871c7" + }, + { + "severity": "major", + "location": { + "path": "src/somefile.c", + "positions": { + "begin": { + "line": 82, + "column": 1 + } + } + }, + "description": "Coverity: Coding standard violation (MISRA C-2012 Rule 10.1)", + "fingerprint": "53754fd4b92326dc35bee26e774c9df3" } ] From 01f26107954d1c902cc66214585e1663529a7f21 Mon Sep 17 00:00:00 2001 From: JWM Date: Thu, 17 Oct 2024 15:48:33 +0200 Subject: [PATCH 5/7] Make test_out folder before running tests --- tox.ini | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tox.ini b/tox.ini index 53069902..b229d873 100644 --- a/tox.ini +++ b/tox.ini @@ -32,7 +32,9 @@ deps = pytest-cov setuptools_scm coverage +allowlist_externals = mkdir commands = + mkdir tests/test_out pytest --cov=mlx --cov-report=term-missing -vv tests/ mlx-warnings -h mlx-warnings --version From da6a926ef4af08bba8bc99065304d2dd75683655 Mon Sep 17 00:00:00 2001 From: JWM Date: Thu, 17 Oct 2024 15:54:02 +0200 Subject: [PATCH 6/7] Fix flake8 --- tests/test_coverity.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_coverity.py b/tests/test_coverity.py index d562b74e..5ff43ef7 100644 --- a/tests/test_coverity.py +++ b/tests/test_coverity.py @@ -10,6 +10,7 @@ TEST_IN_DIR = Path(__file__).parent / 'test_in' TEST_OUT_DIR = Path(__file__).parent / 'test_out' + class TestCoverityWarnings(TestCase): def setUp(self): self.warnings = WarningsPlugin(verbose=True) From efa0956865a7ead60ee02ffac4eebdbb928b4f98 Mon Sep 17 00:00:00 2001 From: JWM Date: Thu, 17 Oct 2024 18:10:53 +0200 Subject: [PATCH 7/7] Make dir if it does not already exists --- src/mlx/warnings/warnings.py | 3 +++ tox.ini | 2 -- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/mlx/warnings/warnings.py b/src/mlx/warnings/warnings.py index bdfbba26..b8c6ec12 100644 --- a/src/mlx/warnings/warnings.py +++ b/src/mlx/warnings/warnings.py @@ -219,6 +219,7 @@ def write_counted_warnings(self, out_file): Args: out_file (str): Location for the output file ''' + Path(out_file).parent.mkdir(parents=True, exist_ok=True) with open(out_file, 'w', encoding='utf-8', newline='\n') as open_file: for checker in self.activated_checkers.values(): open_file.write("\n".join(checker.counted_warnings) + "\n") @@ -233,6 +234,8 @@ def write_code_quality_report(self, out_file): for checker in self.activated_checkers.values(): results.extend(checker.cq_findings) content = json.dumps(results, indent=4, sort_keys=False) + + Path(out_file).parent.mkdir(parents=True, exist_ok=True) with open(out_file, 'w', encoding='utf-8', newline='\n') as open_file: open_file.write(f"{content}\n") diff --git a/tox.ini b/tox.ini index b229d873..53069902 100644 --- a/tox.ini +++ b/tox.ini @@ -32,9 +32,7 @@ deps = pytest-cov setuptools_scm coverage -allowlist_externals = mkdir commands = - mkdir tests/test_out pytest --cov=mlx --cov-report=term-missing -vv tests/ mlx-warnings -h mlx-warnings --version