[analyzer] SATest: Introduce a single entrypoint for regression scripts

[NoQ]

clang/utils/analyzer/CmpRuns.py

526–529

Should we also chown -x these scripts?

[vsavchenko]

clang/utils/analyzer/CmpRuns.py

526–529

That is a great idea!

[Szelethus]

I failed to follow much of this project so my question may be dumb -- but have you considered other run comparing tools such as csa-testbench (https://github.com/Xazax-hun/csa-testbench) and CodeChecker?

[vsavchenko]

In D81567#2085622, @Szelethus wrote:

I failed to follow much of this project so my question may be dumb -- but have you considered other run comparing tools such as csa-testbench (https://github.com/Xazax-hun/csa-testbench) and CodeChecker?

Hi, no you didn't miss anything. I am getting ready ("Eye of the Tiger" starts playing) to address this topic in the mailing list, so I kept you all under informed. These commits is my attempt in making existing scripts that already live in the repo more usable and prettier.

In short, this whole so-to-speak project is about creating a testing system that will for sure produce exactly the same results on any machine. The trick is in using docker to run the analyzer and have all the dependencies pre-installed there.
On csa-testbench and CodeChecker, unfortunately not all of the CSA developers use CodeChecker, and shouldn't be forced to do that for testing.

I will send an email tomorrow and I hope it would make sense :)

[vsavchenko]

Change script modes as the shouldn't be used as executables now

clang/utils/analyzer/CmpRuns.py

Usage:
     resultsA = load_results_from_single_run(singleRunInfoA, delete_empty)
     resultsB = load_results_from_single_run(singleRunInfoB, delete_empty)
 
     # Generate a relation from diagnostics in run A to diagnostics in run B
     # to obtain a list of triples (a, b, confidence).
     diff = compare_results(resultsA, resultsB)
 
 """
-import argparse
 import json
 import os
 import plistlib
 import re
 import sys
 
 from math import log
 from collections import defaultdict
if auxLog:
         auxLog.write(f"('TOTAL DIFFERENCES', {found_diffs})\n")
         auxLog.close()
 
     # TODO: change to NamedTuple
     return found_diffs, len(results_old.diagnostics), \
         len(results_new.diagnostics)
 
 
-def generate_option_parser():
-    parser = argparse.ArgumentParser()
-
-    parser.add_argument("--root-old", dest="root_old",
-                        help="Prefix to ignore on source files for "
-                        "OLD directory",
-                        action="store", type=str, default="")
-    parser.add_argument("--root-new", dest="root_new",
-                        help="Prefix to ignore on source files for "
-                        "NEW directory",
-                        action="store", type=str, default="")
-    parser.add_argument("--verbose-log", dest="verbose_log",
-                        help="Write additional information to LOG "
-                        "[default=None]",
-                        action="store", type=str, default=None,
-                        metavar="LOG")
-    parser.add_argument("--stats-only", action="store_true", dest="stats_only",
-                        default=False, help="Only show statistics on reports")
-    parser.add_argument("--show-stats", action="store_true", dest="show_stats",
-                        default=False, help="Show change in statistics")
-    parser.add_argument("--histogram", action="store", default=None,
-                        choices=[HistogramType.RELATIVE.value,
-                                 HistogramType.LOG_RELATIVE.value,
-                                 HistogramType.ABSOLUTE.value],
-                        help="Show histogram of paths differences. "
-                        "Requires matplotlib")
-    parser.add_argument("old", nargs=1, help="Directory with old results")
-    parser.add_argument("new", nargs=1, help="Directory with new results")
-
-    return parser
-
-
-def main():
-    parser = generate_option_parser()
-    args = parser.parse_args()
-
-    dir_old = ResultsDirectory(args.old[0], args.root_old)
-    dir_new = ResultsDirectory(args.new[0], args.root_new)
-
-    dump_scan_build_results_diff(dir_old, dir_new,
-                                 show_stats=args.show_stats,
-                                 stats_only=args.stats_only,
-                                 histogram=args.histogram,
-                                 verbose_log=args.verbose_log)
-
-
-if __name__ == '__main__':
-    main()
+if __name__ == "__main__":
+    print("CmpRuns.py should not be used on its own.")
+    print("Please use 'SATest.py compare' instead")
+    sys.exit(1)

clang/utils/analyzer/SATest.py

+#!/usr/bin/env python
+
+import SATestAdd
+import SATestBuild
+import SATestUpdateDiffs
+import CmpRuns
+
+from ProjectMap import ProjectInfo, ProjectMap
+
+import argparse
+import sys
+
+
+def add(parser, args):
+    if args.source == "git" and (args.origin == "" or args.commit == ""):
+        parser.error(
+            "Please provide both --origin and --commit if source is 'git'")
+
+    if args.source != "git" and (args.origin != "" or args.commit != ""):
+        parser.error("Options --origin and --commit don't make sense when "
+                     "source is not 'git'")
+
+    project = ProjectInfo(args.name[0], args.mode, args.source, args.origin,
+                          args.commit)
+
+    SATestAdd.add_new_project(project)
+
+
+def build(parser, args):
+    SATestBuild.VERBOSE = args.verbose
+    tester = SATestBuild.RegressionTester(args.jobs, args.override_compiler,
+                                          args.extra_analyzer_config,
+                                          args.regenerate,
+                                          args.strictness)
+    tests_passed = tester.test_all()
+
+    if not tests_passed:
+        sys.stderr.write("ERROR: Tests failed.\n")
+        sys.exit(42)
+
+
+def compare(parser, args):
+    dir_old = CmpRuns.ResultsDirectory(args.old[0], args.root_old)
+    dir_new = CmpRuns.ResultsDirectory(args.new[0], args.root_new)
+
+    CmpRuns.dump_scan_build_results_diff(dir_old, dir_new,
+                                         show_stats=args.show_stats,
+                                         stats_only=args.stats_only,
+                                         histogram=args.histogram,
+                                         verbose_log=args.verbose_log)
+
+
+def update(parser, args):
+    project_map = ProjectMap()
+    for project in project_map.projects:
+        SATestUpdateDiffs.update_reference_results(project)
+
+
+def main():
+    parser = argparse.ArgumentParser()
+    subparsers = parser.add_subparsers()
+
+    # add subcommand
+    add_parser = subparsers.add_parser(
+        "add",
+        help="Add a new project for the analyzer testing.")
+    # TODO: Add an option not to build.
+    # TODO: Set the path to the Repository directory.
+    add_parser.add_argument("name", nargs=1, help="Name of the new project")
+    add_parser.add_argument("--mode", action="store", default=1, type=int,
+                            choices=[0, 1, 2],
+                            help="Build mode: 0 for single file project, "
+                            "1 for scan_build, "
+                            "2 for single file c++11 project")
+    add_parser.add_argument("--source", action="store", default="script",
+                            choices=["script", "git", "zip"],
+                            help="Source type of the new project: "
+                            "'git' for getting from git "
+                            "(please provide --origin and --commit), "
+                            "'zip' for unpacking source from a zip file, "
+                            "'script' for downloading source by running "
+                            "a custom script {}"
+                            .format(SATestBuild.DOWNLOAD_SCRIPT))
+    add_parser.add_argument("--origin", action="store", default="",
+                            help="Origin link for a git repository")
+    add_parser.add_argument("--commit", action="store", default="",
+                            help="Git hash for a commit to checkout")
+    add_parser.set_defaults(func=add)
+
+    # build subcommand
+    build_parser = subparsers.add_parser(
+        "build",
+        help="Build projects from the project map and compare results with "
+        "the reference.")
+    build_parser.add_argument("--strictness", dest="strictness",
+                              type=int, default=0,
+                              help="0 to fail on runtime errors, 1 to fail "
+                              "when the number of found bugs are different "
+                              "from the reference, 2 to fail on any "
+                              "difference from the reference. Default is 0.")
+    build_parser.add_argument("-r", dest="regenerate", action="store_true",
+                              default=False,
+                              help="Regenerate reference output.")
+    build_parser.add_argument("--override-compiler", action="store_true",
+                              default=False, help="Call scan-build with "
+                              "--override-compiler option.")
+    build_parser.add_argument("-j", "--jobs", dest="jobs",
+                              type=int, default=0,
+                              help="Number of projects to test concurrently")
+    build_parser.add_argument("--extra-analyzer-config",
+                              dest="extra_analyzer_config", type=str,
+                              default="",
+                              help="Arguments passed to to -analyzer-config")
+    build_parser.add_argument("-v", "--verbose", action="count", default=0)
+    build_parser.set_defaults(func=build)
+
+    # compare subcommand
+    cmp_parser = subparsers.add_parser(
+        "compare",
+        help="Comparing two static analyzer runs in terms of "
+        "reported warnings and execution time statistics.")
+    cmp_parser.add_argument("--root-old", dest="root_old",
+                            help="Prefix to ignore on source files for "
+                            "OLD directory",
+                            action="store", type=str, default="")
+    cmp_parser.add_argument("--root-new", dest="root_new",
+                            help="Prefix to ignore on source files for "
+                            "NEW directory",
+                            action="store", type=str, default="")
+    cmp_parser.add_argument("--verbose-log", dest="verbose_log",
+                            help="Write additional information to LOG "
+                            "[default=None]",
+                            action="store", type=str, default=None,
+                            metavar="LOG")
+    cmp_parser.add_argument("--stats-only", action="store_true",
+                            dest="stats_only", default=False,
+                            help="Only show statistics on reports")
+    cmp_parser.add_argument("--show-stats", action="store_true",
+                            dest="show_stats", default=False,
+                            help="Show change in statistics")
+    cmp_parser.add_argument("--histogram", action="store", default=None,
+                            choices=[CmpRuns.HistogramType.RELATIVE.value,
+                                     CmpRuns.HistogramType.LOG_RELATIVE.value,
+                                     CmpRuns.HistogramType.ABSOLUTE.value],
+                            help="Show histogram of paths differences. "
+                            "Requires matplotlib")
+    cmp_parser.add_argument("old", nargs=1, help="Directory with old results")
+    cmp_parser.add_argument("new", nargs=1, help="Directory with new results")
+    cmp_parser.set_defaults(func=compare)
+
+    # update subcommand
+    upd_parser = subparsers.add_parser(
+        "update",
+        help="Update static analyzer reference results based on the previous "
+        "run of SATest build. Assumes that SATest build was just run.")
+    # TODO: add option to decide whether we should use git
+    upd_parser.set_defaults(func=update)
+
+    args = parser.parse_args()
+    args.func(parser, args)
+
+
+if __name__ == "__main__":
+    main()

clang/utils/analyzer/SATestAdd.py

The project should use the following files for set up:
                                      source.
                                      Then run:
                                           diff -ur CachedSource PatchedSource \
                                               > changes_for_analyzer.patch
 """
 import SATestBuild
 from ProjectMap import ProjectMap, ProjectInfo
 
-import argparse
 import os
 import sys
 
 
 def add_new_project(project: ProjectInfo):
     """
     Add a new project for testing: build it and add to the Project Map file.
     :param name: is a short string used to identify a project.
else:
         project_map.save()
 
 
 def is_existing_project(project_map: ProjectMap, project: ProjectInfo) -> bool:
     return any(existing_project.name == project.name
                for existing_project in project_map.projects)
 
 
-# TODO: Add an option not to build.
-# TODO: Set the path to the Repository directory.
 if __name__ == "__main__":
-    parser = argparse.ArgumentParser()
-
-    parser.add_argument("name", nargs=1, help="Name of the new project")
-    parser.add_argument("--mode", action="store", default=1, type=int,
-                        choices=[0, 1, 2],
-                        help="Build mode: 0 for single file project, "
-                        "1 for scan_build, "
-                        "2 for single file c++11 project")
-    parser.add_argument("--source", action="store", default="script",
-                        choices=["script", "git", "zip"],
-                        help=f"Source type of the new project: "
-                        f"'git' for getting from git "
-                        f"(please provide --origin and --commit), "
-                        f"'zip' for unpacking source from a zip file, "
-                        f"'script' for downloading source by running "
-                        f"a custom script {SATestBuild.DOWNLOAD_SCRIPT}")
-    parser.add_argument("--origin", action="store", default="",
-                        help="Origin link for a git repository")
-    parser.add_argument("--commit", action="store", default="",
-                        help="Git hash for a commit to checkout")
-
-    args = parser.parse_args()
-
-    if args.source == "git" and (args.origin == "" or args.commit == ""):
-        parser.error(
-            "Please provide both --origin and --commit if source is 'git'")
-
-    if args.source != "git" and (args.origin != "" or args.commit != ""):
-        parser.error("Options --origin and --commit don't make sense when "
-                     "source is not 'git'")
-
-    project = ProjectInfo(args.name[0], args.mode, args.source, args.origin,
-                          args.commit)
-
-    add_new_project(project)
+    print("SATestAdd.py should not be used on its own.")
+    print("Please use 'SATest.py add' instead")
+    sys.exit(1)

clang/utils/analyzer/SATestBuild.py

 The list of checkers tested are hardcoded in the Checkers variable.
 For testing additional checkers, use the SA_ADDITIONAL_CHECKERS environment
 variable. It should contain a comma separated list.
 """
 import CmpRuns
 import SATestUtils
 from ProjectMap import DownloadType, ProjectInfo, ProjectMap
 
-import argparse
 import glob
 import logging
 import math
 import multiprocessing
 import os
 import plistlib
 import shutil
 import sys
class TestProjectThread(threading.Thread):
     def __init__(self, tasks_queue: TestQueue,
                  results_differ: threading.Event,
                  failure_flag: threading.Event):
         """
         :param results_differ: Used to signify that results differ from
                the canonical ones.
         :param failure_flag: Used to signify a failure during the run.
         """
-        self.args = args
         self.tasks_queue = tasks_queue
         self.results_differ = results_differ
         self.failure_flag = failure_flag
         super().__init__()
 
         # Needed to gracefully handle interrupts with Ctrl-C
         self.daemon = True
 
def clean_up_empty_folders(output_dir: str):
     """
     subdirs = glob.glob(output_dir + "/*")
     for subdir in subdirs:
         if not os.listdir(subdir):
             os.removedirs(subdir)
 
 
 if __name__ == "__main__":
-    # Parse command line arguments.
-    parser = argparse.ArgumentParser(
-        description="Test the Clang Static Analyzer.")
-
-    parser.add_argument("--strictness", dest="strictness", type=int, default=0,
-                        help="0 to fail on runtime errors, 1 to fail when the "
-                        "number of found bugs are different from the "
-                        "reference, 2 to fail on any difference from the "
-                        "reference. Default is 0.")
-    parser.add_argument("-r", dest="regenerate", action="store_true",
-                        default=False, help="Regenerate reference output.")
-    parser.add_argument("--override-compiler", action="store_true",
-                        default=False, help="Call scan-build with "
-                        "--override-compiler option.")
-    parser.add_argument("-j", "--jobs", dest="jobs", type=int,
-                        default=0,
-                        help="Number of projects to test concurrently")
-    parser.add_argument("--extra-analyzer-config",
-                        dest="extra_analyzer_config", type=str,
-                        default="",
-                        help="Arguments passed to to -analyzer-config")
-    parser.add_argument("-v", "--verbose", action="count", default=0)
-
-    args = parser.parse_args()
-
-    VERBOSE = args.verbose
-    tester = RegressionTester(args.jobs, args.override_compiler,
-                              args.extra_analyzer_config, args.regenerate,
-                              args.strictness)
-    tests_passed = tester.test_all()
-
-    if not tests_passed:
-        stderr("ERROR: Tests failed.")
-        sys.exit(42)
+    print("SATestBuild.py should not be used on its own.")
+    print("Please use 'SATest.py build' instead")
+    sys.exit(1)