diff --git a/Makefile.am b/Makefile.am index 491b4c8f9f..845440b56f 100644 --- a/Makefile.am +++ b/Makefile.am @@ -183,6 +183,7 @@ EXTRA_DIST+= \ scripts/maint/practracker/testdata/ex0.txt \ scripts/maint/practracker/testdata/ex1-expected.txt \ scripts/maint/practracker/testdata/ex1.txt \ + scripts/maint/practracker/testdata/ex1-overbroad-expected.txt \ scripts/maint/practracker/testdata/ex.txt \ scripts/maint/practracker/testdata/header.h \ scripts/maint/practracker/testdata/not_c_file \ diff --git a/changes/ticket31338 b/changes/ticket31338 new file mode 100644 index 0000000000..b76add635d --- /dev/null +++ b/changes/ticket31338 @@ -0,0 +1,4 @@ + o Minor bugfixes (best practices tracker): + - When listing overbroad exceptions, do not also list problems, + and do not list insufficiently broad exceptions. Fixes bug 31338; + bugfix on 0.4.2.1-alpha. diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py index b5a68fd27c..f6aac9d15e 100755 --- a/scripts/maint/practracker/practracker.py +++ b/scripts/maint/practracker/practracker.py @@ -224,6 +224,11 @@ def main(argv): filt.addThreshold(problem.DependencyViolationItem("*.c", int(args.max_dependency_violations))) filt.addThreshold(problem.DependencyViolationItem("*.h", int(args.max_dependency_violations))) + if args.list_overbroad and args.regen: + print("Cannot use --regen with --list-overbroad", + file=sys.stderr) + sys.exit(1) + # 1) Get all the .c files we care about files_list = util.get_tor_c_files(TOR_TOPDIR, args.include_dir) @@ -239,6 +244,10 @@ def main(argv): ProblemVault = problem.ProblemVault(exceptions_file) problem_file = sys.stdout + if args.list_overbroad: + # If we're listing overbroad exceptions, don't list problems. + problem_file = util.NullFile() + # 2.1) Adjust the exceptions so that we warn only about small problems, # and produce errors on big ones. if not (args.regen or args.list_overbroad or args.strict): diff --git a/scripts/maint/practracker/problem.py b/scripts/maint/practracker/problem.py index 88e1044fb7..d21840a213 100644 --- a/scripts/maint/practracker/problem.py +++ b/scripts/maint/practracker/problem.py @@ -77,8 +77,10 @@ class ProblemVault(object): # (e.g. we went from 4k LoC to 3k LoC), but we do warn if the # situation worsened (e.g. we went from 60 includes to 80). status = problem.is_worse_than(self.exceptions[problem.key()]) - if status == STATUS_OK: - self.used_exception_for[problem.key()] = problem + + # Remember that we used this exception, so that we can later + # determine whether the exception was overbroad. + self.used_exception_for[problem.key()] = problem return status diff --git a/scripts/maint/practracker/test_practracker.sh b/scripts/maint/practracker/test_practracker.sh index bfbd0c6560..9b107e071d 100755 --- a/scripts/maint/practracker/test_practracker.sh +++ b/scripts/maint/practracker/test_practracker.sh @@ -61,3 +61,9 @@ echo "ex1:" run_practracker --exceptions "${DATA}/ex1.txt" > "${TMPDIR}/ex1-received.txt" compare "${TMPDIR}/ex1-received.txt" "${DATA}/ex1-expected.txt" + +echo "ex1.overbroad:" + +run_practracker --exceptions "${DATA}/ex1.txt" --list-overbroad > "${TMPDIR}/ex1-overbroad-received.txt" + +compare "${TMPDIR}/ex1-overbroad-received.txt" "${DATA}/ex1-overbroad-expected.txt" diff --git a/scripts/maint/practracker/testdata/ex1-overbroad-expected.txt b/scripts/maint/practracker/testdata/ex1-overbroad-expected.txt new file mode 100644 index 0000000000..f69c608f40 --- /dev/null +++ b/scripts/maint/practracker/testdata/ex1-overbroad-expected.txt @@ -0,0 +1,2 @@ +problem file-size a.c 40 -> 38 +problem file-size z.c 100 -> 0 diff --git a/scripts/maint/practracker/testdata/ex1.txt b/scripts/maint/practracker/testdata/ex1.txt index f619e33b22..c698005d07 100644 --- a/scripts/maint/practracker/testdata/ex1.txt +++ b/scripts/maint/practracker/testdata/ex1.txt @@ -1,5 +1,5 @@ -problem file-size a.c 38 +problem file-size a.c 40 problem include-count a.c 4 # this problem will produce an error problem function-size a.c:i_am_a_function() 8 @@ -8,6 +8,9 @@ problem function-size a.c:another_function() 11 problem file-size b.c 15 # This is removed, and so will produce an error. # problem function-size b.c:foo() 4 +# This exception isn't used. +problem file-size z.c 100 + problem function-size b.c:bar() 5 problem dependency-violation a.c 3 problem dependency-violation header.h 3 diff --git a/scripts/maint/practracker/util.py b/scripts/maint/practracker/util.py index 4b42565528..df629110c2 100644 --- a/scripts/maint/practracker/util.py +++ b/scripts/maint/practracker/util.py @@ -41,3 +41,10 @@ def get_tor_c_files(tor_topdir, include_dirs=None): files_list.append(full_path) return files_list + +class NullFile: + """A file-like object that we can us to suppress output.""" + def __init__(self): + pass + def write(self, s): + pass