From 0b0f6e509fba1c1751eb9e4e3a6a3cea152b132b Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Tue, 13 Jun 2023 15:23:45 +0200 Subject: [PATCH] Stmt: Rework assertion hooks break semantics Using break in either of the hooks allows to suppress the default reporter error message rather than suppressing solely based on the existence of an assertion_failure() handler. --- scripts/base/init-bare.zeek | 15 +++++++++------ src/Stmt.cc | 14 ++++++++++---- testing/btest/Baseline/language.assert-hook-2/out | 8 ++++---- .../btest/Baseline/language.assert-hook-3/.stderr | 1 + testing/btest/Baseline/language.assert-hook-5/out | 12 ++++++------ .../btest/Baseline/language.assert-hook-6/.stderr | 2 ++ .../btest/Baseline/language.assert-hook-7/.stderr | 2 -- .../btest/Baseline/language.assert-hook/.stderr | 1 + testing/btest/language/assert-hook.zeek | 11 +++++++---- 9 files changed, 40 insertions(+), 26 deletions(-) diff --git a/scripts/base/init-bare.zeek b/scripts/base/init-bare.zeek index 353d667677..ec809377de 100644 --- a/scripts/base/init-bare.zeek +++ b/scripts/base/init-bare.zeek @@ -944,11 +944,10 @@ type Backtrace: vector of BacktraceElement; ## A hook that is invoked when an assert statement fails. ## -## If no such hook is implemented, a default reporter error message is -## logged similar to how scripting errors are reported. -## -## If assertion_failure hook handlers exist, the default reporter error -## message is suppressed to allow for customized error messaging. +## By default, a reporter error message is logged describing the failing +## assert similarly to how scripting errors are reported after invoking +## this hook. Using the :zeek:see:`break` statement in an assertion_failure +## hook handler allows to suppress this message. ## ## cond: The string representation of the condition. ## @@ -964,7 +963,11 @@ type assertion_failure: hook(cond: string, msg: string, bt: Backtrace); ## ## This is a potentially expensive hook meant to be used by testing ## frameworks to summarize assert results. In a production setup, -## this hook is likely deterimental to performance. +## this hook is likely detrimental to performance. +## +## Using the :zeek:see:`break` statement within an assertion_failure hook +## handler allows to suppress the reporter error message generated for +## failing assert statements. ## ## result: The result of evaluating **cond**. ## diff --git a/src/Stmt.cc b/src/Stmt.cc index 060a1ea710..acdad6b8f8 100644 --- a/src/Stmt.cc +++ b/src/Stmt.cc @@ -1928,16 +1928,22 @@ ValPtr AssertStmt::Exec(Frame* f, StmtFlowType& flow) bt->Insert(0, assert_elem); } + // Breaking from either the assertion_failure() or assertion_result() + // hook can be used to suppress the default log message. + bool report_error = true; + if ( run_result_hook ) - assertion_result_hook->Invoke(zeek::val_mgr->Bool(assert_result), cond_val, msg_val, bt); + report_error &= assertion_result_hook + ->Invoke(zeek::val_mgr->Bool(assert_result), cond_val, msg_val, bt) + ->AsBool(); if ( assert_result ) return Val::nil; - // Run the installed failure hooks, or log a default message. if ( run_failure_hook ) - assertion_failure_hook->Invoke(cond_val, msg_val, bt); - else + report_error &= assertion_failure_hook->Invoke(cond_val, msg_val, bt)->AsBool(); + + if ( report_error ) { std::string reporter_msg = util::fmt("assertion failure: %s", cond_val->CheckString()); if ( msg_val->Len() > 0 ) diff --git a/testing/btest/Baseline/language.assert-hook-2/out b/testing/btest/Baseline/language.assert-hook-2/out index 8fb8a43cb8..f898a52732 100644 --- a/testing/btest/Baseline/language.assert-hook-2/out +++ b/testing/btest/Baseline/language.assert-hook-2/out @@ -1,7 +1,7 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. assertion_failure, to_count("5") == 4, 5 is not 4 -assert <...>/assert-hook.zeek:19 - f <...>/assert-hook.zeek:23 - g <...>/assert-hook.zeek:24 - h <...>/assert-hook.zeek:28 +assert <...>/assert-hook.zeek:21 + f <...>/assert-hook.zeek:25 + g <...>/assert-hook.zeek:26 + h <...>/assert-hook.zeek:30 zeek_init :0 diff --git a/testing/btest/Baseline/language.assert-hook-3/.stderr b/testing/btest/Baseline/language.assert-hook-3/.stderr index e3f6131b1d..a1707e3408 100644 --- a/testing/btest/Baseline/language.assert-hook-3/.stderr +++ b/testing/btest/Baseline/language.assert-hook-3/.stderr @@ -1,2 +1,3 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +error in <...>/assert-hook.zeek, line 12: assertion failure: F (terminate me!) received termination signal diff --git a/testing/btest/Baseline/language.assert-hook-5/out b/testing/btest/Baseline/language.assert-hook-5/out index 30e53dff7b..cfd12f766e 100644 --- a/testing/btest/Baseline/language.assert-hook-5/out +++ b/testing/btest/Baseline/language.assert-hook-5/out @@ -1,8 +1,8 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -assertion_result T at <...>/assert-hook.zeek:24: md5_hash("") == "d41d8cd98f00b204e9800998ecf8427e" -assertion_result T at <...>/assert-hook.zeek:29: sha1_hash("") == "da39a3ee5e6b4b0d3255bfef95601890afd80709" -assertion_result F at <...>/assert-hook.zeek:34: sha1_hash("") == "wrong" -assertion_failure at <...>/assert-hook.zeek:34: sha1_hash("") == "wrong" -assertion_result F at <...>/assert-hook.zeek:39: md5_hash("") == "wrong" -assertion_failure at <...>/assert-hook.zeek:39: md5_hash("") == "wrong" +assertion_result T at <...>/assert-hook.zeek:25: md5_hash("") == "d41d8cd98f00b204e9800998ecf8427e" +assertion_result T at <...>/assert-hook.zeek:30: sha1_hash("") == "da39a3ee5e6b4b0d3255bfef95601890afd80709" +assertion_result F at <...>/assert-hook.zeek:35: sha1_hash("") == "wrong" +assertion_failure at <...>/assert-hook.zeek:35: sha1_hash("") == "wrong" +assertion_result F at <...>/assert-hook.zeek:40: md5_hash("") == "wrong" +assertion_failure at <...>/assert-hook.zeek:40: md5_hash("") == "wrong" 2 of 4 assertions failed diff --git a/testing/btest/Baseline/language.assert-hook-6/.stderr b/testing/btest/Baseline/language.assert-hook-6/.stderr index bf9daba5a9..13dbb41dae 100644 --- a/testing/btest/Baseline/language.assert-hook-6/.stderr +++ b/testing/btest/Baseline/language.assert-hook-6/.stderr @@ -1,3 +1,5 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. expression error in <...>/assert-hook.zeek, line 15: field value missing (get_current_packet_header()$ip) expression error in <...>/assert-hook.zeek, line 17: field value missing (get_current_packet_header()$ip) +error in <...>/assert-hook.zeek, line 17: assertion failure: 2 + 2 == 5 () +error in <...>/assert-hook.zeek, line 22: assertion failure: 2 + 2 == 5 ({"msg":"false and works"}) diff --git a/testing/btest/Baseline/language.assert-hook-7/.stderr b/testing/btest/Baseline/language.assert-hook-7/.stderr index 824abcadef..49d861c74c 100644 --- a/testing/btest/Baseline/language.assert-hook-7/.stderr +++ b/testing/btest/Baseline/language.assert-hook-7/.stderr @@ -1,3 +1 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -error in <...>/assert-hook.zeek, line 12: assertion failure: 2 + 2 == 5 (this is false) -error in <...>/assert-hook.zeek, line 18: assertion failure: 2 + 2 == 5 (this is false) diff --git a/testing/btest/Baseline/language.assert-hook/.stderr b/testing/btest/Baseline/language.assert-hook/.stderr index 49d861c74c..70a0f9c3a1 100644 --- a/testing/btest/Baseline/language.assert-hook/.stderr +++ b/testing/btest/Baseline/language.assert-hook/.stderr @@ -1 +1,2 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +error in <...>/assert-hook.zeek, line 15: assertion failure: 1 != 1 diff --git a/testing/btest/language/assert-hook.zeek b/testing/btest/language/assert-hook.zeek index c7126f6518..1edca8ee6b 100644 --- a/testing/btest/language/assert-hook.zeek +++ b/testing/btest/language/assert-hook.zeek @@ -4,7 +4,7 @@ # @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff out # @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderr -# Hook calls break after logging out some information. +# Hook is not calling break: Reporter log is produced. hook assertion_failure(cond: string, msg: string, bt: Backtrace) { print "assertion_failure", cond, msg, bt[0]$file_location, bt[0]$line_location; @@ -17,7 +17,7 @@ event zeek_init() } @TEST-START-NEXT -# Test the backtrace location +# Test the backtrace location, also calling break to suppress reporter log. hook assertion_failure(cond: string, msg: string, bt: Backtrace) { print "assertion_failure", cond, msg; @@ -29,6 +29,8 @@ hook assertion_failure(cond: string, msg: string, bt: Backtrace) print fmt("%s%s %s:%s", indent, e$function_name, file_name, line_number); indent = fmt("%s ", indent); } + + break; } @@ -102,6 +104,7 @@ hook assertion_failure(cond: string, msg: string, bt: Backtrace) cond, |msg| > 0 ? " - " : "", msg); ++assertion_failures; + break; } hook assertion_result(result: bool, cond: string, msg: string, bt: Backtrace) @@ -170,11 +173,11 @@ event zeek_done() } @TEST-START-NEXT -# Only implementing assertion_result() falls back to default -# reporter errors. +# Breaking in assertion_result() also suppresses the reporter errors. hook assertion_result(result: bool, cond: string, msg: string, bt: Backtrace) { print "assertion_result", result, cond, msg, bt[0]$file_location, bt[0]$line_location; + break; } event zeek_init()