From c71051882586201f9f2566fa7073e13bf50b60db Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 26 Aug 2019 12:28:46 -0400 Subject: [PATCH 1/5] Add integration tests for new practracker features These tests check our .may_include checking, and our header file checking. They do not pass yet: we have a bug in our filtering code. --- scripts/maint/practracker/test_practracker.sh | 4 +++- scripts/maint/practracker/testdata/.may_include | 3 +++ scripts/maint/practracker/testdata/a.c | 2 +- scripts/maint/practracker/testdata/ex0-expected.txt | 4 ++++ scripts/maint/practracker/testdata/ex1.txt | 4 ++++ scripts/maint/practracker/testdata/header.h | 8 ++++++++ 6 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 scripts/maint/practracker/testdata/.may_include create mode 100644 scripts/maint/practracker/testdata/header.h diff --git a/scripts/maint/practracker/test_practracker.sh b/scripts/maint/practracker/test_practracker.sh index c878ca5580..4f8b7e2047 100755 --- a/scripts/maint/practracker/test_practracker.sh +++ b/scripts/maint/practracker/test_practracker.sh @@ -25,7 +25,9 @@ DATA="${PRACTRACKER_DIR}/testdata" run_practracker() { "${PYTHON:-python}" "${PRACTRACKER_DIR}/practracker.py" \ - --max-include-count=0 --max-file-size=0 --max-function-size=0 --terse \ + --max-include-count=0 --max-file-size=0 \ + --max-h-include-count=0 --max-h-file-size=0 \ + --max-function-size=0 --terse \ "${DATA}/" "$@"; } compare() { diff --git a/scripts/maint/practracker/testdata/.may_include b/scripts/maint/practracker/testdata/.may_include new file mode 100644 index 0000000000..40bf8155d9 --- /dev/null +++ b/scripts/maint/practracker/testdata/.may_include @@ -0,0 +1,3 @@ +!advisory + +permitted.h diff --git a/scripts/maint/practracker/testdata/a.c b/scripts/maint/practracker/testdata/a.c index b52a14f56a..1939773f57 100644 --- a/scripts/maint/practracker/testdata/a.c +++ b/scripts/maint/practracker/testdata/a.c @@ -3,7 +3,7 @@ #include "two.h" #incldue "three.h" -# include "four.h" +# include "permitted.h" int i_am_a_function(void) diff --git a/scripts/maint/practracker/testdata/ex0-expected.txt b/scripts/maint/practracker/testdata/ex0-expected.txt index c021e6f710..5f3d9e5aec 100644 --- a/scripts/maint/practracker/testdata/ex0-expected.txt +++ b/scripts/maint/practracker/testdata/ex0-expected.txt @@ -2,6 +2,10 @@ problem file-size a.c 38 problem include-count a.c 4 problem function-size a.c:i_am_a_function() 9 problem function-size a.c:another_function() 12 +problem dependency-violation a.c 3 problem file-size b.c 15 problem function-size b.c:foo() 4 problem function-size b.c:bar() 5 +problem file-size header.h 8 +problem include-count header.h 4 +problem dependency-violation header.h 3 diff --git a/scripts/maint/practracker/testdata/ex1.txt b/scripts/maint/practracker/testdata/ex1.txt index db42ae8450..f619e33b22 100644 --- a/scripts/maint/practracker/testdata/ex1.txt +++ b/scripts/maint/practracker/testdata/ex1.txt @@ -9,3 +9,7 @@ problem file-size b.c 15 # This is removed, and so will produce an error. # problem function-size b.c:foo() 4 problem function-size b.c:bar() 5 +problem dependency-violation a.c 3 +problem dependency-violation header.h 3 +problem file-size header.h 8 +problem include-count header.h 4 diff --git a/scripts/maint/practracker/testdata/header.h b/scripts/maint/practracker/testdata/header.h new file mode 100644 index 0000000000..1183f5db9a --- /dev/null +++ b/scripts/maint/practracker/testdata/header.h @@ -0,0 +1,8 @@ + +// some forbidden includes +#include "foo.h" +#include "quux.h" +#include "quup.h" + +// a permitted include +#include "permitted.h" From 318de94e49c99335987bfdead899c29908afc5bc Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 26 Aug 2019 12:30:18 -0400 Subject: [PATCH 2/5] Fix a bug in practracker's handling of .may_include in headers I was expecting our filter code to work in a way it didn't. I thought that saying that DependencyViolation applied to "*" would hit all of the files -- but actually, "*" wasn't implemented. I had to say "*.c" and "*.h" --- scripts/maint/practracker/practracker.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py index 6483b88da1..b280a76765 100755 --- a/scripts/maint/practracker/practracker.py +++ b/scripts/maint/practracker/practracker.py @@ -213,7 +213,8 @@ def main(argv): filt.addThreshold(problem.FileSizeItem("*.h", int(args.max_h_file_size))) filt.addThreshold(problem.IncludeCountItem("*.h", int(args.max_h_include_count))) filt.addThreshold(problem.FunctionSizeItem("*.c", int(args.max_function_size))) - filt.addThreshold(problem.DependencyViolationItem("*", int(args.max_dependency_violations))) + filt.addThreshold(problem.DependencyViolationItem("*.c", int(args.max_dependency_violations))) + filt.addThreshold(problem.DependencyViolationItem("*.h", int(args.max_dependency_violations))) # 1) Get all the .c files we care about files_list = util.get_tor_c_files(TOR_TOPDIR) From bc4ddbf4aced574c6729220a924a38bfe1b0b63e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 26 Aug 2019 12:33:44 -0400 Subject: [PATCH 3/5] New practracker exceptions for dependency violations in headers I've done this manually, since I don't want to override the existing exceptions in this branch. --- scripts/maint/practracker/exceptions.txt | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt index 0acb6fb7f7..f0306ebeba 100644 --- a/scripts/maint/practracker/exceptions.txt +++ b/scripts/maint/practracker/exceptions.txt @@ -325,3 +325,12 @@ problem function-size /src/tools/tor-gencert.c:parse_commandline() 111 problem function-size /src/tools/tor-resolve.c:build_socks5_resolve_request() 102 problem function-size /src/tools/tor-resolve.c:do_resolve() 171 problem function-size /src/tools/tor-resolve.c:main() 112 + +problem dependency-violation /scripts/maint/practracker/testdata/a.c 3 +problem dependency-violation /scripts/maint/practracker/testdata/header.h 3 +problem dependency-violation /src/core/crypto/hs_ntor.h 1 +problem dependency-violation /src/core/or/cell_queue_st.h 1 +problem dependency-violation /src/core/or/channel.h 1 +problem dependency-violation /src/core/or/circuitlist.h 1 +problem dependency-violation /src/core/or/connection_edge.h 1 +problem dependency-violation /src/core/or/or.h 1 From 884ae485f6b0bb73b23cf246cc4cc2e0615b54c0 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 26 Aug 2019 13:47:09 -0400 Subject: [PATCH 4/5] Add new practracker test files to Makefile.am --- Makefile.am | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Makefile.am b/Makefile.am index d3cce3934d..dd5bf904b2 100644 --- a/Makefile.am +++ b/Makefile.am @@ -174,6 +174,7 @@ EXTRA_DIST+= \ scripts/maint/practracker/practracker.py \ scripts/maint/practracker/practracker_tests.py \ scripts/maint/practracker/problem.py \ + scripts/maint/practracker/testdata/.may_include \ scripts/maint/practracker/testdata/a.c \ scripts/maint/practracker/testdata/b.c \ scripts/maint/practracker/testdata/ex0-expected.txt \ @@ -181,6 +182,7 @@ EXTRA_DIST+= \ scripts/maint/practracker/testdata/ex1-expected.txt \ scripts/maint/practracker/testdata/ex1.txt \ scripts/maint/practracker/testdata/ex.txt \ + scripts/maint/practracker/testdata/header.h \ scripts/maint/practracker/testdata/not_c_file \ scripts/maint/practracker/test_practracker.sh \ scripts/maint/practracker/util.py From 380d178e53bf4389a4f3085aef73d23c4a6b447f Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 5 Sep 2019 16:20:31 -0400 Subject: [PATCH 5/5] changes file for ticket31477 --- changes/ticket31477 | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changes/ticket31477 diff --git a/changes/ticket31477 b/changes/ticket31477 new file mode 100644 index 0000000000..5a0fdd1544 --- /dev/null +++ b/changes/ticket31477 @@ -0,0 +1,3 @@ + o Minor features (tests): + - Add integration tests to make sure that practracker gives the outputs + we expect. Closes ticket 31477.