From f336aa5084c58dfa6aac45999078a8c004c19b31 Mon Sep 17 00:00:00 2001 From: Daniel Thayer Date: Mon, 22 May 2017 14:32:59 -0500 Subject: [PATCH 1/6] Fix a race condition in some failing tests The tests that were using the broccoli-v6addrs test program would sometimes fail because broccoli-v6addrs would try to connect to Bro and fail (presumably because Bro hadn't yet fully initialized). Fixed by using the new broccoli-v6addrs "-r" option which will retry upon failure to connect to Bro. --- testing/btest/istate/broccoli-ipv6-socket.bro | 3 +-- testing/btest/istate/broccoli-ipv6.bro | 3 +-- testing/btest/istate/broccoli-ssl.bro | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/testing/btest/istate/broccoli-ipv6-socket.bro b/testing/btest/istate/broccoli-ipv6-socket.bro index 7365999bb0..0c8f1392cb 100644 --- a/testing/btest/istate/broccoli-ipv6-socket.bro +++ b/testing/btest/istate/broccoli-ipv6-socket.bro @@ -4,8 +4,7 @@ # @TEST-REQUIRES: ifconfig | grep -q -E "inet6 ::1|inet6 addr: ::1" # # @TEST-EXEC: btest-bg-run bro bro $DIST/aux/broccoli/test/broccoli-v6addrs.bro "Communication::listen_ipv6=T" -# @TEST-EXEC: sleep 3 -# @TEST-EXEC: btest-bg-run broccoli $BUILD/aux/broccoli/test/broccoli-v6addrs -6 ::1 +# @TEST-EXEC: btest-bg-run broccoli $BUILD/aux/broccoli/test/broccoli-v6addrs -r -6 ::1 # @TEST-EXEC: btest-bg-wait 20 # @TEST-EXEC: btest-diff bro/.stdout # @TEST-EXEC: btest-diff broccoli/.stdout diff --git a/testing/btest/istate/broccoli-ipv6.bro b/testing/btest/istate/broccoli-ipv6.bro index b4fdfb5fcf..8d56ef196f 100644 --- a/testing/btest/istate/broccoli-ipv6.bro +++ b/testing/btest/istate/broccoli-ipv6.bro @@ -3,8 +3,7 @@ # @TEST-REQUIRES: test -e $BUILD/aux/broccoli/src/libbroccoli.so || test -e $BUILD/aux/broccoli/src/libbroccoli.dylib # # @TEST-EXEC: btest-bg-run bro bro $DIST/aux/broccoli/test/broccoli-v6addrs.bro -# @TEST-EXEC: sleep 1 -# @TEST-EXEC: btest-bg-run broccoli $BUILD/aux/broccoli/test/broccoli-v6addrs +# @TEST-EXEC: btest-bg-run broccoli $BUILD/aux/broccoli/test/broccoli-v6addrs -r # @TEST-EXEC: btest-bg-wait 20 # @TEST-EXEC: btest-diff bro/.stdout # @TEST-EXEC: btest-diff broccoli/.stdout diff --git a/testing/btest/istate/broccoli-ssl.bro b/testing/btest/istate/broccoli-ssl.bro index 13b17a0b7b..ba510bf595 100644 --- a/testing/btest/istate/broccoli-ssl.bro +++ b/testing/btest/istate/broccoli-ssl.bro @@ -4,8 +4,7 @@ # # @TEST-EXEC: chmod 600 broccoli.conf # @TEST-EXEC: btest-bg-run bro bro $DIST/aux/broccoli/test/broccoli-v6addrs.bro "Communication::listen_ssl=T" "ssl_ca_certificate=../ca_cert.pem" "ssl_private_key=../bro.pem" -# @TEST-EXEC: sleep 5 -# @TEST-EXEC: btest-bg-run broccoli BROCCOLI_CONFIG_FILE=../broccoli.conf $BUILD/aux/broccoli/test/broccoli-v6addrs +# @TEST-EXEC: btest-bg-run broccoli BROCCOLI_CONFIG_FILE=../broccoli.conf $BUILD/aux/broccoli/test/broccoli-v6addrs -r # @TEST-EXEC: btest-bg-wait 20 # @TEST-EXEC: btest-diff bro/.stdout # @TEST-EXEC: btest-diff broccoli/.stdout From 34551dda155114c2ac989a3b301f55e9e328fb10 Mon Sep 17 00:00:00 2001 From: Daniel Thayer Date: Wed, 24 May 2017 13:10:26 -0500 Subject: [PATCH 2/6] The broccoli-v6addrs "-r" option was renamed to "-R" --- testing/btest/istate/broccoli-ipv6-socket.bro | 2 +- testing/btest/istate/broccoli-ipv6.bro | 2 +- testing/btest/istate/broccoli-ssl.bro | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/testing/btest/istate/broccoli-ipv6-socket.bro b/testing/btest/istate/broccoli-ipv6-socket.bro index 0c8f1392cb..27df984471 100644 --- a/testing/btest/istate/broccoli-ipv6-socket.bro +++ b/testing/btest/istate/broccoli-ipv6-socket.bro @@ -4,7 +4,7 @@ # @TEST-REQUIRES: ifconfig | grep -q -E "inet6 ::1|inet6 addr: ::1" # # @TEST-EXEC: btest-bg-run bro bro $DIST/aux/broccoli/test/broccoli-v6addrs.bro "Communication::listen_ipv6=T" -# @TEST-EXEC: btest-bg-run broccoli $BUILD/aux/broccoli/test/broccoli-v6addrs -r -6 ::1 +# @TEST-EXEC: btest-bg-run broccoli $BUILD/aux/broccoli/test/broccoli-v6addrs -R -6 ::1 # @TEST-EXEC: btest-bg-wait 20 # @TEST-EXEC: btest-diff bro/.stdout # @TEST-EXEC: btest-diff broccoli/.stdout diff --git a/testing/btest/istate/broccoli-ipv6.bro b/testing/btest/istate/broccoli-ipv6.bro index 8d56ef196f..0e360df713 100644 --- a/testing/btest/istate/broccoli-ipv6.bro +++ b/testing/btest/istate/broccoli-ipv6.bro @@ -3,7 +3,7 @@ # @TEST-REQUIRES: test -e $BUILD/aux/broccoli/src/libbroccoli.so || test -e $BUILD/aux/broccoli/src/libbroccoli.dylib # # @TEST-EXEC: btest-bg-run bro bro $DIST/aux/broccoli/test/broccoli-v6addrs.bro -# @TEST-EXEC: btest-bg-run broccoli $BUILD/aux/broccoli/test/broccoli-v6addrs -r +# @TEST-EXEC: btest-bg-run broccoli $BUILD/aux/broccoli/test/broccoli-v6addrs -R # @TEST-EXEC: btest-bg-wait 20 # @TEST-EXEC: btest-diff bro/.stdout # @TEST-EXEC: btest-diff broccoli/.stdout diff --git a/testing/btest/istate/broccoli-ssl.bro b/testing/btest/istate/broccoli-ssl.bro index ba510bf595..fce5ed8535 100644 --- a/testing/btest/istate/broccoli-ssl.bro +++ b/testing/btest/istate/broccoli-ssl.bro @@ -4,7 +4,7 @@ # # @TEST-EXEC: chmod 600 broccoli.conf # @TEST-EXEC: btest-bg-run bro bro $DIST/aux/broccoli/test/broccoli-v6addrs.bro "Communication::listen_ssl=T" "ssl_ca_certificate=../ca_cert.pem" "ssl_private_key=../bro.pem" -# @TEST-EXEC: btest-bg-run broccoli BROCCOLI_CONFIG_FILE=../broccoli.conf $BUILD/aux/broccoli/test/broccoli-v6addrs -r +# @TEST-EXEC: btest-bg-run broccoli BROCCOLI_CONFIG_FILE=../broccoli.conf $BUILD/aux/broccoli/test/broccoli-v6addrs -R # @TEST-EXEC: btest-bg-wait 20 # @TEST-EXEC: btest-diff bro/.stdout # @TEST-EXEC: btest-diff broccoli/.stdout From 961c247777261def79b3917d02ebb381f1712fb3 Mon Sep 17 00:00:00 2001 From: Daniel Thayer Date: Wed, 24 May 2017 13:13:20 -0500 Subject: [PATCH 3/6] Fix a race condition in some failing tests Use the new "-R" option for broccoli-vectors and broping so that they will retry connecting to Bro until the connection is established. This avoids a race condition and eliminates the need for a "sleep" after starting Bro. --- testing/btest/istate/broccoli-vector.bro | 3 +-- testing/btest/istate/broccoli.bro | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/testing/btest/istate/broccoli-vector.bro b/testing/btest/istate/broccoli-vector.bro index ce107f45d3..cf0b0c8642 100644 --- a/testing/btest/istate/broccoli-vector.bro +++ b/testing/btest/istate/broccoli-vector.bro @@ -3,8 +3,7 @@ # @TEST-REQUIRES: test -e $BUILD/aux/broccoli/src/libbroccoli.so || test -e $BUILD/aux/broccoli/src/libbroccoli.dylib # # @TEST-EXEC: btest-bg-run bro bro $DIST/aux/broccoli/test/broccoli-vectors.bro -# @TEST-EXEC: sleep 1 -# @TEST-EXEC: btest-bg-run broccoli $BUILD/aux/broccoli/test/broccoli-vectors +# @TEST-EXEC: btest-bg-run broccoli $BUILD/aux/broccoli/test/broccoli-vectors -R # @TEST-EXEC: btest-bg-wait 20 # @TEST-EXEC: btest-diff bro/.stdout # @TEST-EXEC: btest-diff broccoli/.stdout diff --git a/testing/btest/istate/broccoli.bro b/testing/btest/istate/broccoli.bro index 2fdd4cbda4..a6427412bf 100644 --- a/testing/btest/istate/broccoli.bro +++ b/testing/btest/istate/broccoli.bro @@ -3,8 +3,7 @@ # @TEST-REQUIRES: test -e $BUILD/aux/broccoli/src/libbroccoli.so || test -e $BUILD/aux/broccoli/src/libbroccoli.dylib # # @TEST-EXEC: btest-bg-run bro bro %INPUT $DIST/aux/broccoli/test/broping-record.bro -# @TEST-EXEC: sleep 1 -# @TEST-EXEC: btest-bg-run broccoli $BUILD/aux/broccoli/test/broping -r -c 3 127.0.0.1 +# @TEST-EXEC: btest-bg-run broccoli $BUILD/aux/broccoli/test/broping -R -r -c 3 127.0.0.1 # @TEST-EXEC: btest-bg-wait 20 # @TEST-EXEC: cat bro/ping.log | sed 's/one-way.*//g' >bro.log # @TEST-EXEC: cat broccoli/.stdout | sed 's/time=.*//g' >broccoli.log From bd2d559fbf52a8243219a1de9856b685a9cb893a Mon Sep 17 00:00:00 2001 From: Daniel Thayer Date: Wed, 24 May 2017 14:29:24 -0500 Subject: [PATCH 4/6] Fix race condition causing some tests to fail Removed loading of the "frameworks/communication/listen" script for a couple of tests that don't need this functionality. This was causing failures of some broccoli-related tests in the "istate" test directory due to two instances of Bro trying to listen on the same port. --- .../Baseline/language.expire-expr-error/output | 2 +- testing/btest/language/expire-expr-error.bro | 14 ++++++-------- testing/btest/language/expire-redef.bro | 2 -- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/testing/btest/Baseline/language.expire-expr-error/output b/testing/btest/Baseline/language.expire-expr-error/output index 544527fe23..cf43dd4c80 100644 --- a/testing/btest/Baseline/language.expire-expr-error/output +++ b/testing/btest/Baseline/language.expire-expr-error/output @@ -1,2 +1,2 @@ -error in /home/robin/bro/master/testing/btest/.tmp/language.expire-expr-error/expire-expr-error.bro, line 7: no such index (x[kaputt]) +error in /home/robin/bro/master/testing/btest/.tmp/language.expire-expr-error/expire-expr-error.bro, line 8: no such index (x[kaputt]) received termination signal diff --git a/testing/btest/language/expire-expr-error.bro b/testing/btest/language/expire-expr-error.bro index c355bd58ed..7c9a3aa318 100644 --- a/testing/btest/language/expire-expr-error.bro +++ b/testing/btest/language/expire-expr-error.bro @@ -1,13 +1,12 @@ -# @TEST-EXEC: btest-bg-run broproc bro %INPUT -# @TEST-EXEC: btest-bg-wait -k 5 -# @TEST-EXEC: cat broproc/.stderr > output +# @TEST-EXEC: bro -b %INPUT +# @TEST-EXEC: cp .stderr output # @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff output +redef exit_only_after_terminate = T; + global x: table[string] of interval; global data: table[int] of string &create_expire=x["kaputt"]; -@load frameworks/communication/listen - global runs = 0; event do_it() { @@ -16,6 +15,8 @@ event do_it() ++runs; if ( runs < 4 ) schedule 1sec { do_it() }; + else + terminate(); } @@ -24,6 +25,3 @@ event bro_init() &priority=-10 data[0] = "some data"; schedule 1sec { do_it() }; } - - - diff --git a/testing/btest/language/expire-redef.bro b/testing/btest/language/expire-redef.bro index f08ac8d3f2..6bf43ae98a 100644 --- a/testing/btest/language/expire-redef.bro +++ b/testing/btest/language/expire-redef.bro @@ -3,8 +3,6 @@ redef exit_only_after_terminate = T; -@load frameworks/communication/listen - const exp_val = -1sec &redef; global expired: function(tbl: table[int] of string, idx: int): interval; From 361a5dc2d8eb27e1f6b88f7fe4fbbae5c4a2e62a Mon Sep 17 00:00:00 2001 From: Daniel Thayer Date: Wed, 24 May 2017 21:25:01 -0500 Subject: [PATCH 5/6] Serialize tests that load listen.bro Tests that load "frameworks/communication/listen" must be serialized to prevent other tests failing due to multiple Bro instances trying to listen on the same port. --- testing/btest/doc/broxygen/identifier.bro | 1 + testing/btest/doc/broxygen/package.bro | 1 + testing/btest/doc/broxygen/package_index.bro | 1 + testing/btest/doc/broxygen/script_index.bro | 1 + testing/btest/doc/broxygen/script_summary.bro | 1 + 5 files changed, 5 insertions(+) diff --git a/testing/btest/doc/broxygen/identifier.bro b/testing/btest/doc/broxygen/identifier.bro index 3768b0c0c6..db5c2528ee 100644 --- a/testing/btest/doc/broxygen/identifier.bro +++ b/testing/btest/doc/broxygen/identifier.bro @@ -1,3 +1,4 @@ +# @TEST-SERIALIZE: comm # @TEST-EXEC: bro -b -X broxygen.config %INPUT # @TEST-EXEC: btest-diff test.rst diff --git a/testing/btest/doc/broxygen/package.bro b/testing/btest/doc/broxygen/package.bro index 6857d5e646..fd75a1ce21 100644 --- a/testing/btest/doc/broxygen/package.bro +++ b/testing/btest/doc/broxygen/package.bro @@ -1,3 +1,4 @@ +# @TEST-SERIALIZE: comm # @TEST-EXEC: bro -b -X broxygen.config %INPUT # @TEST-EXEC: btest-diff test.rst diff --git a/testing/btest/doc/broxygen/package_index.bro b/testing/btest/doc/broxygen/package_index.bro index e29479d49f..ef6cc4ab29 100644 --- a/testing/btest/doc/broxygen/package_index.bro +++ b/testing/btest/doc/broxygen/package_index.bro @@ -1,3 +1,4 @@ +# @TEST-SERIALIZE: comm # @TEST-EXEC: bro -b -X broxygen.config %INPUT # @TEST-EXEC: btest-diff test.rst diff --git a/testing/btest/doc/broxygen/script_index.bro b/testing/btest/doc/broxygen/script_index.bro index 91bb4b756f..86e1909863 100644 --- a/testing/btest/doc/broxygen/script_index.bro +++ b/testing/btest/doc/broxygen/script_index.bro @@ -1,3 +1,4 @@ +# @TEST-SERIALIZE: comm # @TEST-EXEC: bro -b -X broxygen.config %INPUT # @TEST-EXEC: btest-diff test.rst diff --git a/testing/btest/doc/broxygen/script_summary.bro b/testing/btest/doc/broxygen/script_summary.bro index 9d3cda012b..a7aafc65a0 100644 --- a/testing/btest/doc/broxygen/script_summary.bro +++ b/testing/btest/doc/broxygen/script_summary.bro @@ -1,3 +1,4 @@ +# @TEST-SERIALIZE: comm # @TEST-EXEC: bro -b -X broxygen.config %INPUT # @TEST-EXEC: btest-diff test.rst From e9102f3de4d261eba3a7d9a54b4f673c73989beb Mon Sep 17 00:00:00 2001 From: Daniel Thayer Date: Wed, 24 May 2017 21:28:56 -0500 Subject: [PATCH 6/6] Remove loading of listen.bro in tests that do not need it Removed the loading of "frameworks/communication/listen" from some tests that don't need that functionality. This is to avoid serializing these tests. --- testing/btest/scripts/base/frameworks/intel/expire-item.bro | 5 ++++- testing/btest/scripts/base/frameworks/intel/match-subnet.bro | 2 +- .../scripts/base/frameworks/intel/remove-non-existing.bro | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/testing/btest/scripts/base/frameworks/intel/expire-item.bro b/testing/btest/scripts/base/frameworks/intel/expire-item.bro index dd915b3a03..690a461ea4 100644 --- a/testing/btest/scripts/base/frameworks/intel/expire-item.bro +++ b/testing/btest/scripts/base/frameworks/intel/expire-item.bro @@ -9,9 +9,10 @@ 1.2.3.4 Intel::ADDR source1 this host is bad http://some-data-distributor.com/1 # @TEST-END-FILE -@load frameworks/communication/listen @load frameworks/intel/do_expire +redef exit_only_after_terminate = T; + redef Intel::read_files += { "../intel.dat" }; redef enum Intel::Where += { SOMEWHERE }; redef Intel::item_expiration = 9sec; @@ -44,6 +45,8 @@ event do_it() if ( runs < 6 ) schedule 3sec { do_it() }; + else + terminate(); } event Intel::match(s: Intel::Seen, items: set[Intel::Item]) diff --git a/testing/btest/scripts/base/frameworks/intel/match-subnet.bro b/testing/btest/scripts/base/frameworks/intel/match-subnet.bro index 1e25868de1..8e3fe74116 100644 --- a/testing/btest/scripts/base/frameworks/intel/match-subnet.bro +++ b/testing/btest/scripts/base/frameworks/intel/match-subnet.bro @@ -14,7 +14,7 @@ 192.168.128.0/18 Intel::SUBNET source1 this subnetwork might be baaad http://some-data-distributor.com/5 # @TEST-END-FILE -@load frameworks/communication/listen +redef exit_only_after_terminate = T; redef Intel::read_files += { "../intel.dat" }; redef enum Intel::Where += { SOMEWHERE }; diff --git a/testing/btest/scripts/base/frameworks/intel/remove-non-existing.bro b/testing/btest/scripts/base/frameworks/intel/remove-non-existing.bro index 379d132834..1885f5bcf8 100644 --- a/testing/btest/scripts/base/frameworks/intel/remove-non-existing.bro +++ b/testing/btest/scripts/base/frameworks/intel/remove-non-existing.bro @@ -9,7 +9,7 @@ 192.168.1.1 Intel::ADDR source1 this host is just plain baaad http://some-data-distributor.com/1 # @TEST-END-FILE -@load frameworks/communication/listen +redef exit_only_after_terminate = T; redef Intel::read_files += { "../intel.dat" }; redef enum Intel::Where += { SOMEWHERE };