move MISRA mutation tests into test.sh (#2304)

* move MISRA mutation tests into test.sh

* no sed

* lil cleanup

* this stuff is slow for the dumbest reasons

* sample

.github/workflows/tests.yml

@@ -36,20 +36,6 @@ jobs:
     - name: Run safety tests
       run: ./opendbc/safety/tests/test.sh ${{ matrix.flags }}
 
-  misra_mutation:
-    name: MISRA C:2012 Mutation
-    runs-on: ${{ github.repository == 'commaai/opendbc' && 'namespace-profile-amd64-8x16' || 'ubuntu-latest' }}
-    timeout-minutes: 20
-    steps:
-      - uses: actions/checkout@v4
-      - uses: ./.github/workflows/cache
-      - name: MISRA mutation tests
-        timeout-minutes: 1
-        run: |
-          source setup.sh
-          scons -j8
-          pytest -s -n8 --randomly-seed $RANDOM opendbc/safety/tests/misra/test_mutation.py
-
   mutation:
     name: Safety mutation tests
     runs-on: ${{ github.repository == 'commaai/opendbc' && 'namespace-profile-amd64-8x16' || 'ubuntu-latest' }}

conftest.py

@@ -1,4 +1,5 @@
 # pytest attempts to execute shell scripts while collecting
 collect_ignore_glob = [
-  "opendbc/safety/tests/misra/*",
+  "opendbc/safety/tests/misra/*.sh",
+  "opendbc/safety/tests/misra/cppcheck/",
 ]

opendbc/safety/tests/misra/install.sh

@@ -4,6 +4,11 @@ set -e
 DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
 : "${CPPCHECK_DIR:=$DIR/cppcheck/}"
 
+# skip if we're running in parallel with test_mutation.py
+if [ ! -z "$OPENDBC_ROOT" ]; then
+  exit 0
+fi
+
 if [ ! -d "$CPPCHECK_DIR" ]; then
   git clone https://github.com/danmar/cppcheck.git $CPPCHECK_DIR
 fi

opendbc/safety/tests/misra/test_misra.sh

@@ -14,20 +14,15 @@ NC='\033[0m'
 : "${CPPCHECK_DIR:=$DIR/cppcheck/}"
 
 # ensure checked in coverage table is up to date
-if [ -z "$SKIP_TABLES_DIFF" ]; then
-  python3 $CPPCHECK_DIR/addons/misra.py -generate-table > coverage_table
-  if ! git diff --quiet coverage_table; then
-    echo -e "${YELLOW}MISRA coverage table doesn't match. Update and commit:${NC}"
-    exit 3
-  fi
+python3 $CPPCHECK_DIR/addons/misra.py -generate-table > coverage_table
+if ! git diff --quiet coverage_table; then
+  echo -e "${YELLOW}MISRA coverage table doesn't match. Update and commit:${NC}"
+  exit 3
 fi
 
 cd $BASEDIR
-if [ -z "${SKIP_BUILD}" ]; then
-  scons -j8
-fi
 
-CHECKLIST=$DIR/checkers.txt
+CHECKLIST=$(mktemp)
 echo "Cppcheck checkers list from test_misra.sh:" > $CHECKLIST
 
 cppcheck() {
@@ -35,12 +30,13 @@ cppcheck() {
   COMMON_DEFINES="-D__GNUC__=9"
 
   # note that cppcheck build cache results in inconsistent results as of v2.13.0
-  OUTPUT=$DIR/.output.log
+  OUTPUT=$(mktemp)
 
   echo -e "\n\n\n\n\nTEST variant options:" >> $CHECKLIST
   echo -e ""${@//$BASEDIR/}"\n\n" >> $CHECKLIST # (absolute path removed)
 
-  $CPPCHECK_DIR/cppcheck --inline-suppr -I $BASEDIR \
+  OPENDBC_ROOT=${OPENDBC_ROOT:-$BASEDIR}
+  $CPPCHECK_DIR/cppcheck --inline-suppr -I $OPENDBC_ROOT \
           -I "$(gcc -print-file-name=include)" --suppress=*:*gcc*include/* --suppress=*:*clang*include/* \
           --suppressions-list=$DIR/suppressions.txt  \
            --error-exitcode=2 --check-level=exhaustive --safety \
@@ -65,8 +61,11 @@ cppcheck $PANDA_OPTS -DCANFD $BASEDIR/opendbc/safety/main.c
 printf "\n${GREEN}Success!${NC} took $SECONDS seconds\n"
 
 # ensure list of checkers is up to date
-cd $DIR
-if [ -z "$SKIP_TABLES_DIFF" ] && ! git diff --quiet $CHECKLIST; then
-  echo -e "\n${YELLOW}WARNING: Cppcheck checkers.txt report has changed. Review and commit...${NC}"
-  exit 4
+if [ -z "$OPENDBC_ROOT" ]; then
+  cd $DIR
+  if ! git diff --quiet $CHECKLIST; then
+    echo -e "\n${YELLOW}WARNING: Cppcheck checkers.txt report has changed. Review and commit...${NC}"
+    mv $CHECKLIST $DIR/checkers.txt
+    exit 4
+  fi
 fi

opendbc/safety/tests/misra/test_mutation.py

@@ -11,38 +11,27 @@ HERE = os.path.abspath(os.path.dirname(__file__))
 ROOT = os.path.join(HERE, "../../../../")
 
 IGNORED_PATHS = (
+  'opendbc/safety/main.c',
   'opendbc/safety/tests/',
   'opendbc/safety/board/',
 )
 
 mutations = [
-  # default
-  (None, None, False),
-  # general safety
-  ("opendbc/safety/modes/toyota.h", "s/if (addr == 0x260) {/if (addr == 1 || addr == 2) {/g", True),
+  # no mutation, should pass
+  (None, None, lambda s: s, False),
 ]
 
 patterns = [
-  # misra-c2012-13.3
-  "$a void test(int tmp) { int tmp2 = tmp++ + 2; if (tmp2) {;}}",
-  # misra-c2012-13.4
-  "$a int test(int x, int y) { return (x=2) && (y=2); }",
-  # misra-c2012-13.5
-  "$a void test(int tmp) { if (true && tmp++) {;} }",
-  # misra-c2012-13.6
-  "$a void test(int tmp) { if (sizeof(tmp++)) {;} }",
-  # misra-c2012-14.1
-  "$a void test(float len) { for (float j = 0; j < len; j++) {;} }",
-  # misra-c2012-14.4
-  "$a void test(int len) { if (len - 8) {;} }",
-  # misra-c2012-16.4
-  r"$a void test(int temp) {switch (temp) { case 1: ; }}\n",
-  # misra-c2012-17.8
-  "$a void test(int cnt) { for (cnt=0;;cnt++) {;} }",
-  # misra-c2012-20.4
-  r"$a #define auto 1\n",
-  # misra-c2012-20.5
-  r"$a #define TEST 1\n#undef TEST\n",
+  ("misra-c2012-10.3", lambda s: s + "\nvoid test(float len) { for (float j = 0; j < len; j++) {;} }\n"),
+  ("misra-c2012-13.3", lambda s: s + "\nvoid test(int tmp) { int tmp2 = tmp++ + 2; if (tmp2) {;}}\n"),
+  ("misra-c2012-13.4", lambda s: s + "\nint test(int x, int y) { return (x=2) && (y=2); }\n"),
+  ("misra-c2012-13.5", lambda s: s + "\nvoid test(int tmp) { if (true && tmp++) {;} }\n"),
+  ("misra-c2012-13.6", lambda s: s + "\nvoid test(int tmp) { if (sizeof(tmp++)) {;} }\n"),
+  ("misra-c2012-14.2", lambda s: s + "\nvoid test(int cnt) { for (cnt=0;;cnt++) {;} }\n"),
+  ("misra-c2012-14.4", lambda s: s + "\nvoid test(int len) { if (len - 8) {;} }\n"),
+  ("misra-c2012-16.4", lambda s: s + "\nvoid test(int temp) {switch (temp) { case 1: ; }}\n"),
+  ("misra-c2012-20.4", lambda s: s + "\n#define auto 1\n"),
+  ("misra-c2012-20.5", lambda s: s + "\n#define TEST 1\n#undef TEST\n"),
 ]
 
 all_files = glob.glob('opendbc/safety/**', root_dir=ROOT, recursive=True)
@@ -50,20 +39,28 @@ files = [f for f in all_files if f.endswith(('.c', '.h')) and not f.startswith(I
 assert len(files) > 20, files
 
 for p in patterns:
-  mutations.append((random.choice(files), p, True))
+  mutations.append((random.choice(files), *p, True))
 
-@pytest.mark.parametrize("fn, patch, should_fail", mutations)
-def test_misra_mutation(fn, patch, should_fail):
+mutations = random.sample(mutations, 2)  # can remove this once cppcheck is faster
+
+@pytest.mark.parametrize("fn, rule, transform, should_fail", mutations)
+def test_misra_mutation(fn, rule, transform, should_fail):
   with tempfile.TemporaryDirectory() as tmp:
-    shutil.copytree(ROOT, tmp, dirs_exist_ok=True)
-    shutil.rmtree(os.path.join(tmp, '.venv'), ignore_errors=True)
+    shutil.copytree(ROOT, tmp, dirs_exist_ok=True,
+                    ignore=shutil.ignore_patterns('.venv', 'cppcheck', '.git', '*.ctu-info'))
 
     # apply patch
     if fn is not None:
-      r = os.system(f"cd {tmp} && sed -i '{patch}' {fn}")
-      assert r == 0
+      with open(os.path.join(tmp, fn), 'r+') as f:
+        content = f.read()
+        f.seek(0)
+        f.write(transform(content))
 
     # run test
-    r = subprocess.run("SKIP_TABLES_DIFF=1 SKIP_BUILD=1 opendbc/safety/tests/misra/test_misra.sh", cwd=tmp, shell=True)
+    r = subprocess.run(f"OPENDBC_ROOT={tmp} opendbc/safety/tests/misra/test_misra.sh",
+                       stdout=subprocess.PIPE, cwd=ROOT, shell=True, encoding='utf8')
+    print(r.stdout) # helpful for debugging failures
     failed = r.returncode != 0
     assert failed == should_fail
+    if should_fail:
+      assert rule in r.stdout, "MISRA test failed but not for the correct violation"