Merge pull request #149 from melexis/fix-cq-bug

Fix code quality bug due missing shared object
melexis · Oct 21, 2024 · 11b7799 · 11b7799
2 parents cf2bb39 + efa0956
commit 11b7799
Show file tree

Hide file tree

Showing 11 changed files with 259 additions and 8 deletions.
diff --git a/src/mlx/warnings/polyspace_checker.py b/src/mlx/warnings/polyspace_checker.py
@@ -17,9 +17,16 @@ 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) '''
+        '''List[str]: list of counted warnings'''
         all_counted_warnings = []
         for checker in self.checkers:
             all_counted_warnings.extend(checker.counted_warnings)
@@ -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
@@ -111,12 +111,19 @@ 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)
         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/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.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/src/mlx/warnings/warnings_checker.py b/src/mlx/warnings/warnings_checker.py
@@ -42,16 +42,21 @@ 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) '''
+        ''' List[str]: list of counted warnings'''
         return self._counted_warnings
 
     @property

diff --git a/tests/test_coverity.py b/tests/test_coverity.py
@@ -1,9 +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):
@@ -47,3 +52,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/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
@@ -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/config_example_coverity.yml 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
@@ -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
@@ -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.