From 5ce45ac90750ebe9a5d482eef0d9e4a6eb4fbb38 Mon Sep 17 00:00:00 2001 From: teor Date: Sat, 16 Nov 2019 21:44:44 +1000 Subject: [PATCH 1/5] Makefile/git scripts: Add the practracker unit tests Add a check-practracker-unit-test target, and run it as part of "make check". Add the practracker unit tests to scripts/git/pre-commit.git-hook. Part of 32609. --- Makefile.am | 5 ++++- changes/ticket32609 | 3 +++ scripts/git/pre-commit.git-hook | 13 ++++++++----- 3 files changed, 15 insertions(+), 6 deletions(-) create mode 100644 changes/ticket32609 diff --git a/Makefile.am b/Makefile.am index 38040a4e75..0c3a4fb76d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -253,7 +253,10 @@ test: all shellcheck: $(top_srcdir)/scripts/maint/checkShellScripts.sh -check-local: check-spaces check-changes check-includes check-best-practices shellcheck check-cocci +check-practracker-unit-test: + $(top_srcdir)/scripts/maint/practracker/test_practracker.sh + +check-local: check-spaces check-changes check-includes check-practracker-unit-test check-best-practices shellcheck check-cocci need-chutney-path: @if test ! -d "$$CHUTNEY_PATH"; then \ diff --git a/changes/ticket32609 b/changes/ticket32609 new file mode 100644 index 0000000000..2fc3f733f5 --- /dev/null +++ b/changes/ticket32609 @@ -0,0 +1,3 @@ + o Testing: + - Run the practracker unit tests as part of "make check", and in the + pre-commit git hook. Closes ticket 32609. diff --git a/scripts/git/pre-commit.git-hook b/scripts/git/pre-commit.git-hook index c138d2ae21..c5adfaffe7 100755 --- a/scripts/git/pre-commit.git-hook +++ b/scripts/git/pre-commit.git-hook @@ -54,6 +54,13 @@ if [ -e scripts/maint/checkShellScripts.sh ]; then scripts/maint/checkShellScripts.sh fi +# Always run the practracker unit tests +PT_DIR=scripts/maint/practracker + +if [ -e "${PT_DIR}/test_practracker.sh" ]; then + "${PT_DIR}/test_practracker.sh" +fi + if [ ! "$CHECK_FILES" ]; then echo "No modified tor-owned source files, skipping further checks" exit 0 @@ -75,13 +82,9 @@ fi # Only call practracker if ${PT_DIR}/.enable_practracker_in_hooks exists # We do this check so that we can enable practracker in hooks in master, and # disable it on maint branches -PT_DIR=scripts/maint/practracker - if [ -e "${PT_DIR}/practracker.py" ]; then if [ -e "${PT_DIR}/.enable_practracker_in_hooks" ]; then - if ! python3 "${PT_DIR}/practracker.py" "$workdir"; then - exit 1 - fi + python3 "${PT_DIR}/practracker.py" "$workdir" fi fi From 19324ff0bae96059155a5b16301528d682b78078 Mon Sep 17 00:00:00 2001 From: teor Date: Sat, 16 Nov 2019 21:45:52 +1000 Subject: [PATCH 2/5] practracker: Test for bad ".c" includes in the unit tests Part of 32522. --- scripts/maint/practracker/testdata/.may_include | 1 + scripts/maint/practracker/testdata/a.c | 3 +++ scripts/maint/practracker/testdata/ex0-expected.txt | 6 +++--- .../maint/practracker/testdata/ex1-overbroad-expected.txt | 2 +- scripts/maint/practracker/testdata/ex1.txt | 6 +++--- 5 files changed, 11 insertions(+), 7 deletions(-) diff --git a/scripts/maint/practracker/testdata/.may_include b/scripts/maint/practracker/testdata/.may_include index 40bf8155d9..8542a35807 100644 --- a/scripts/maint/practracker/testdata/.may_include +++ b/scripts/maint/practracker/testdata/.may_include @@ -1,3 +1,4 @@ !advisory permitted.h +ext/good.c diff --git a/scripts/maint/practracker/testdata/a.c b/scripts/maint/practracker/testdata/a.c index 1939773f57..3c338ab40d 100644 --- a/scripts/maint/practracker/testdata/a.c +++ b/scripts/maint/practracker/testdata/a.c @@ -5,6 +5,9 @@ # include "permitted.h" +#include "ext/good.c" +#include "bad.c" + 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 5f3d9e5aec..502005edb1 100644 --- a/scripts/maint/practracker/testdata/ex0-expected.txt +++ b/scripts/maint/practracker/testdata/ex0-expected.txt @@ -1,8 +1,8 @@ -problem file-size a.c 38 -problem include-count a.c 4 +problem file-size a.c 41 +problem include-count a.c 6 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 dependency-violation a.c 4 problem file-size b.c 15 problem function-size b.c:foo() 4 problem function-size b.c:bar() 5 diff --git a/scripts/maint/practracker/testdata/ex1-overbroad-expected.txt b/scripts/maint/practracker/testdata/ex1-overbroad-expected.txt index f69c608f40..b757202dae 100644 --- a/scripts/maint/practracker/testdata/ex1-overbroad-expected.txt +++ b/scripts/maint/practracker/testdata/ex1-overbroad-expected.txt @@ -1,2 +1,2 @@ -problem file-size a.c 40 -> 38 +problem file-size a.c 45 -> 41 problem file-size z.c 100 -> 0 diff --git a/scripts/maint/practracker/testdata/ex1.txt b/scripts/maint/practracker/testdata/ex1.txt index c698005d07..af8de03291 100644 --- a/scripts/maint/practracker/testdata/ex1.txt +++ b/scripts/maint/practracker/testdata/ex1.txt @@ -1,6 +1,6 @@ -problem file-size a.c 40 -problem include-count a.c 4 +problem file-size a.c 45 +problem include-count a.c 6 # this problem will produce an error problem function-size a.c:i_am_a_function() 8 # this problem will produce a warning @@ -12,7 +12,7 @@ problem file-size b.c 15 problem file-size z.c 100 problem function-size b.c:bar() 5 -problem dependency-violation a.c 3 +problem dependency-violation a.c 4 problem dependency-violation header.h 3 problem file-size header.h 8 problem include-count header.h 4 From 8861ad85d02c10dc7b1add8d1721c5ae48c930b8 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 3 Dec 2019 13:06:30 +1000 Subject: [PATCH 3/5] git scripts: Add the checkSpace.pl unit tests Extra testing after 32613. --- scripts/git/pre-commit.git-hook | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/scripts/git/pre-commit.git-hook b/scripts/git/pre-commit.git-hook index c5adfaffe7..95d566b17d 100755 --- a/scripts/git/pre-commit.git-hook +++ b/scripts/git/pre-commit.git-hook @@ -61,6 +61,10 @@ if [ -e "${PT_DIR}/test_practracker.sh" ]; then "${PT_DIR}/test_practracker.sh" fi +if [ -e scripts/maint/checkSpaceTest.sh ]; then + scripts/maint/checkSpaceTest.sh +fi + if [ ! "$CHECK_FILES" ]; then echo "No modified tor-owned source files, skipping further checks" exit 0 From b0f1b760aec8fa25fae34be794285fbdfcaaa7e9 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 3 Dec 2019 13:28:55 +1000 Subject: [PATCH 4/5] practracker: document !advisory in includes.py --- scripts/maint/practracker/includes.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/scripts/maint/practracker/includes.py b/scripts/maint/practracker/includes.py index 1d43bd58e8..239ea19eaf 100755 --- a/scripts/maint/practracker/includes.py +++ b/scripts/maint/practracker/includes.py @@ -13,6 +13,10 @@ file in each directory. This file contains empty lines, #-prefixed comments, filenames (like "lib/foo/bar.h") and file globs (like lib/*/*.h) for files that are permitted. + + The script exits with an error if any non-permitted includes are found. + .may_include files that contain "!advisory" are considered advisory. + Advisory .may_include files only result in warnings, rather than errors. """ From 9ad569c71d081992ac644695064ffb39ce88c61a Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 3 Dec 2019 13:29:19 +1000 Subject: [PATCH 5/5] practracker: Add missing .may_include files All of these files contain "*.h", except for: * src/app/config/.may_include * src/test/.may_include which also contain "*.inc". This change prevents includes of "*.c" files, and other unusually named files. Part of 32609. --- src/app/config/.may_include | 2 ++ src/app/main/.may_include | 1 + src/feature/api/.may_include | 1 + src/feature/client/.may_include | 1 + src/feature/control/.may_include | 1 + src/feature/dirauth/.may_include | 1 + src/feature/dircache/.may_include | 1 + src/feature/dirclient/.may_include | 1 + src/feature/dircommon/.may_include | 1 + src/feature/dirparse/.may_include | 1 + src/feature/hibernate/.may_include | 1 + src/feature/hs/.may_include | 1 + src/feature/hs_common/.may_include | 1 + src/feature/keymgt/.may_include | 1 + src/feature/nodelist/.may_include | 1 + src/feature/relay/.may_include | 1 + src/feature/rend/.may_include | 1 + src/feature/stats/.may_include | 1 + src/test/.may_include | 2 ++ src/test/fuzz/.may_include | 1 + src/tools/.may_include | 1 + src/trunnel/.may_include | 1 + src/trunnel/hs/.may_include | 1 + 23 files changed, 25 insertions(+) create mode 100644 src/app/config/.may_include create mode 100644 src/app/main/.may_include create mode 100644 src/feature/api/.may_include create mode 100644 src/feature/client/.may_include create mode 100644 src/feature/control/.may_include create mode 100644 src/feature/dirauth/.may_include create mode 100644 src/feature/dircache/.may_include create mode 100644 src/feature/dirclient/.may_include create mode 100644 src/feature/dircommon/.may_include create mode 100644 src/feature/dirparse/.may_include create mode 100644 src/feature/hibernate/.may_include create mode 100644 src/feature/hs/.may_include create mode 100644 src/feature/hs_common/.may_include create mode 100644 src/feature/keymgt/.may_include create mode 100644 src/feature/nodelist/.may_include create mode 100644 src/feature/relay/.may_include create mode 100644 src/feature/rend/.may_include create mode 100644 src/feature/stats/.may_include create mode 100644 src/test/.may_include create mode 100644 src/test/fuzz/.may_include create mode 100644 src/tools/.may_include create mode 100644 src/trunnel/.may_include create mode 100644 src/trunnel/hs/.may_include diff --git a/src/app/config/.may_include b/src/app/config/.may_include new file mode 100644 index 0000000000..11c5ffbb14 --- /dev/null +++ b/src/app/config/.may_include @@ -0,0 +1,2 @@ +*.h +*.inc diff --git a/src/app/main/.may_include b/src/app/main/.may_include new file mode 100644 index 0000000000..424c745c12 --- /dev/null +++ b/src/app/main/.may_include @@ -0,0 +1 @@ +*.h diff --git a/src/feature/api/.may_include b/src/feature/api/.may_include new file mode 100644 index 0000000000..424c745c12 --- /dev/null +++ b/src/feature/api/.may_include @@ -0,0 +1 @@ +*.h diff --git a/src/feature/client/.may_include b/src/feature/client/.may_include new file mode 100644 index 0000000000..424c745c12 --- /dev/null +++ b/src/feature/client/.may_include @@ -0,0 +1 @@ +*.h diff --git a/src/feature/control/.may_include b/src/feature/control/.may_include new file mode 100644 index 0000000000..424c745c12 --- /dev/null +++ b/src/feature/control/.may_include @@ -0,0 +1 @@ +*.h diff --git a/src/feature/dirauth/.may_include b/src/feature/dirauth/.may_include new file mode 100644 index 0000000000..424c745c12 --- /dev/null +++ b/src/feature/dirauth/.may_include @@ -0,0 +1 @@ +*.h diff --git a/src/feature/dircache/.may_include b/src/feature/dircache/.may_include new file mode 100644 index 0000000000..424c745c12 --- /dev/null +++ b/src/feature/dircache/.may_include @@ -0,0 +1 @@ +*.h diff --git a/src/feature/dirclient/.may_include b/src/feature/dirclient/.may_include new file mode 100644 index 0000000000..424c745c12 --- /dev/null +++ b/src/feature/dirclient/.may_include @@ -0,0 +1 @@ +*.h diff --git a/src/feature/dircommon/.may_include b/src/feature/dircommon/.may_include new file mode 100644 index 0000000000..424c745c12 --- /dev/null +++ b/src/feature/dircommon/.may_include @@ -0,0 +1 @@ +*.h diff --git a/src/feature/dirparse/.may_include b/src/feature/dirparse/.may_include new file mode 100644 index 0000000000..424c745c12 --- /dev/null +++ b/src/feature/dirparse/.may_include @@ -0,0 +1 @@ +*.h diff --git a/src/feature/hibernate/.may_include b/src/feature/hibernate/.may_include new file mode 100644 index 0000000000..424c745c12 --- /dev/null +++ b/src/feature/hibernate/.may_include @@ -0,0 +1 @@ +*.h diff --git a/src/feature/hs/.may_include b/src/feature/hs/.may_include new file mode 100644 index 0000000000..424c745c12 --- /dev/null +++ b/src/feature/hs/.may_include @@ -0,0 +1 @@ +*.h diff --git a/src/feature/hs_common/.may_include b/src/feature/hs_common/.may_include new file mode 100644 index 0000000000..424c745c12 --- /dev/null +++ b/src/feature/hs_common/.may_include @@ -0,0 +1 @@ +*.h diff --git a/src/feature/keymgt/.may_include b/src/feature/keymgt/.may_include new file mode 100644 index 0000000000..424c745c12 --- /dev/null +++ b/src/feature/keymgt/.may_include @@ -0,0 +1 @@ +*.h diff --git a/src/feature/nodelist/.may_include b/src/feature/nodelist/.may_include new file mode 100644 index 0000000000..424c745c12 --- /dev/null +++ b/src/feature/nodelist/.may_include @@ -0,0 +1 @@ +*.h diff --git a/src/feature/relay/.may_include b/src/feature/relay/.may_include new file mode 100644 index 0000000000..424c745c12 --- /dev/null +++ b/src/feature/relay/.may_include @@ -0,0 +1 @@ +*.h diff --git a/src/feature/rend/.may_include b/src/feature/rend/.may_include new file mode 100644 index 0000000000..424c745c12 --- /dev/null +++ b/src/feature/rend/.may_include @@ -0,0 +1 @@ +*.h diff --git a/src/feature/stats/.may_include b/src/feature/stats/.may_include new file mode 100644 index 0000000000..424c745c12 --- /dev/null +++ b/src/feature/stats/.may_include @@ -0,0 +1 @@ +*.h diff --git a/src/test/.may_include b/src/test/.may_include new file mode 100644 index 0000000000..11c5ffbb14 --- /dev/null +++ b/src/test/.may_include @@ -0,0 +1,2 @@ +*.h +*.inc diff --git a/src/test/fuzz/.may_include b/src/test/fuzz/.may_include new file mode 100644 index 0000000000..424c745c12 --- /dev/null +++ b/src/test/fuzz/.may_include @@ -0,0 +1 @@ +*.h diff --git a/src/tools/.may_include b/src/tools/.may_include new file mode 100644 index 0000000000..424c745c12 --- /dev/null +++ b/src/tools/.may_include @@ -0,0 +1 @@ +*.h diff --git a/src/trunnel/.may_include b/src/trunnel/.may_include new file mode 100644 index 0000000000..424c745c12 --- /dev/null +++ b/src/trunnel/.may_include @@ -0,0 +1 @@ +*.h diff --git a/src/trunnel/hs/.may_include b/src/trunnel/hs/.may_include new file mode 100644 index 0000000000..424c745c12 --- /dev/null +++ b/src/trunnel/hs/.may_include @@ -0,0 +1 @@ +*.h