Message ID | 20210202191207.4103973-1-ehabkost@redhat.com |
---|---|
State | New |
Headers | show |
Series | device-crash-test: Remove problematic language | expand |
On 2/2/21 2:12 PM, Eduardo Habkost wrote: > Replace "whitelist" in the device-crash-test script with > "rule list". > > I'm using "rule list" instead of "allow list" or "pass list" > because the list is not used only for expected/allowed errors. > It also contain rules specifying which errors shouldn't be > ignored and/or should be fatal. > Yeah, I see. It isn't really strictly an allow list. This is indeed more accurately named. The docs clear up nicely the function and syntax of the rules, so this is fine. > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> > --- > scripts/device-crash-test | 96 +++++++++++++++++++-------------------- > 1 file changed, 48 insertions(+), 48 deletions(-) > > diff --git a/scripts/device-crash-test b/scripts/device-crash-test > index 04118669ba7..ef1412ca590 100755 > --- a/scripts/device-crash-test > +++ b/scripts/device-crash-test > @@ -41,18 +41,18 @@ logger = logging.getLogger('device-crash-test') > dbg = logger.debug > > > -# Purposes of the following whitelist: > +# Purposes of the following rule list: > # * Avoiding verbose log messages when we find known non-fatal > # (exitcode=1) errors > # * Avoiding fatal errors when we find known crashes > # * Skipping machines/devices that are known not to work out of > # the box, when running in --quick mode > # > -# Keeping the whitelist updated is desirable, but not required, > +# Keeping the rule list updated is desirable, but not required, > # because unexpected cases where QEMU exits with exitcode=1 will > # just trigger a INFO message. > > -# Valid whitelist entry keys: > +# Valid error rule keys: > # * accel: regexp, full match only > # * machine: regexp, full match only > # * device: regexp, full match only > @@ -62,7 +62,7 @@ dbg = logger.debug > # * expected: if True, QEMU is expected to always fail every time > # when testing the corresponding test case > # * loglevel: log level of log output when there's a match. > -ERROR_WHITELIST = [ > +ERROR_RULE_LIST = [ > # Machines that won't work out of the box: > # MACHINE | ERROR MESSAGE > {'machine':'niagara', 'expected':True}, # Unable to load a firmware for -M niagara > @@ -186,65 +186,65 @@ ERROR_WHITELIST = [ > ] > > > -def whitelistTestCaseMatch(wl, t): > - """Check if a test case specification can match a whitelist entry > +def errorRuleTestCaseMatch(rule, t): > + """Check if a test case specification can match a error rule > > - This only checks if a whitelist entry is a candidate match > + This only checks if a error rule is a candidate match > for a given test case, it won't check if the test case > - results/output match the entry. See whitelistResultMatch(). > + results/output match the rule. See ruleListResultMatch(). > """ > - return (('machine' not in wl or > + return (('machine' not in rule or > 'machine' not in t or > - re.match(wl['machine'] + '$', t['machine'])) and > - ('accel' not in wl or > + re.match(rule['machine'] + '$', t['machine'])) and > + ('accel' not in rule or > 'accel' not in t or > - re.match(wl['accel'] + '$', t['accel'])) and > - ('device' not in wl or > + re.match(rule['accel'] + '$', t['accel'])) and > + ('device' not in rule or > 'device' not in t or > - re.match(wl['device'] + '$', t['device']))) > + re.match(rule['device'] + '$', t['device']))) > > > -def whitelistCandidates(t): > +def ruleListCandidates(t): > """Generate the list of candidates that can match a test case""" > - for i, wl in enumerate(ERROR_WHITELIST): > - if whitelistTestCaseMatch(wl, t): > - yield (i, wl) > + for i, rule in enumerate(ERROR_RULE_LIST): > + if errorRuleTestCaseMatch(rule, t): > + yield (i, rule) > > > def findExpectedResult(t): > - """Check if there's an expected=True whitelist entry for a test case > + """Check if there's an expected=True error rule for a test case > > - Returns (i, wl) tuple, where i is the index in > - ERROR_WHITELIST and wl is the whitelist entry itself. > + Returns (i, rule) tuple, where i is the index in > + ERROR_RULE_LIST and rule is the error rule itself. > """ > - for i, wl in whitelistCandidates(t): > - if wl.get('expected'): > - return (i, wl) > + for i, rule in ruleListCandidates(t): > + if rule.get('expected'): > + return (i, rule) > > > -def whitelistResultMatch(wl, r): > - """Check if test case results/output match a whitelist entry > +def ruleListResultMatch(rule, r): > + """Check if test case results/output match a error rule > > It is valid to call this function only if > - whitelistTestCaseMatch() is True for the entry (e.g. on > - entries returned by whitelistCandidates()) > + errorRuleTestCaseMatch() is True for the rule (e.g. on > + rules returned by ruleListCandidates()) > """ > - assert whitelistTestCaseMatch(wl, r['testcase']) > - return ((wl.get('exitcode', 1) is None or > - r['exitcode'] == wl.get('exitcode', 1)) and > - ('log' not in wl or > - re.search(wl['log'], r['log'], re.MULTILINE))) > + assert errorRuleTestCaseMatch(rule, r['testcase']) > + return ((rule.get('exitcode', 1) is None or > + r['exitcode'] == rule.get('exitcode', 1)) and > + ('log' not in rule or > + re.search(rule['log'], r['log'], re.MULTILINE))) > > > -def checkResultWhitelist(r): > - """Look up whitelist entry for a given test case result > +def checkResultRuleList(r): > + """Look up error rule for a given test case result > > - Returns (i, wl) tuple, where i is the index in > - ERROR_WHITELIST and wl is the whitelist entry itself. > + Returns (i, rule) tuple, where i is the index in > + ERROR_RULE_LIST and rule is the error rule itself. > """ > - for i, wl in whitelistCandidates(r['testcase']): > - if whitelistResultMatch(wl, r): > - return i, wl > + for i, rule in ruleListCandidates(r['testcase']): > + if ruleListResultMatch(rule, r): > + return i, rule > > raise Exception("this should never happen") > > @@ -543,12 +543,12 @@ def main(): > break > > if f: > - i, wl = checkResultWhitelist(f) > - dbg("testcase: %r, whitelist match: %r", t, wl) > + i, rule = checkResultRuleList(f) > + dbg("testcase: %r, rule list match: %r", t, rule) > wl_stats.setdefault(i, []).append(f) > - level = wl.get('loglevel', logging.DEBUG) > + level = rule.get('loglevel', logging.DEBUG) > logFailure(f, level) > - if wl.get('fatal') or (args.strict and level >= logging.WARN): > + if rule.get('fatal') or (args.strict and level >= logging.WARN): > fatal_failures.append(f) > else: > dbg("success: %s", formatTestCase(t)) > @@ -560,10 +560,10 @@ def main(): > logger.info("Skipped %d test cases", skipped) > > if args.debug: > - stats = sorted([(len(wl_stats.get(i, [])), wl) for i, wl in > - enumerate(ERROR_WHITELIST)], key=lambda x: x[0]) > - for count, wl in stats: > - dbg("whitelist entry stats: %d: %r", count, wl) > + stats = sorted([(len(wl_stats.get(i, [])), rule) for i, rule in > + enumerate(ERROR_RULE_LIST)], key=lambda x: x[0]) > + for count, rule in stats: > + dbg("error rule stats: %d: %r", count, rule) > > if fatal_failures: > for f in fatal_failures: >
On 02/02/2021 20.12, Eduardo Habkost wrote: > Replace "whitelist" in the device-crash-test script with > "rule list". > > I'm using "rule list" instead of "allow list" or "pass list" > because the list is not used only for expected/allowed errors. > It also contain rules specifying which errors shouldn't be > ignored and/or should be fatal. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > scripts/device-crash-test | 96 +++++++++++++++++++-------------------- > 1 file changed, 48 insertions(+), 48 deletions(-) Good idea. Acked-by: Thomas Huth <thuth@redhat.com>
diff --git a/scripts/device-crash-test b/scripts/device-crash-test index 04118669ba7..ef1412ca590 100755 --- a/scripts/device-crash-test +++ b/scripts/device-crash-test @@ -41,18 +41,18 @@ logger = logging.getLogger('device-crash-test') dbg = logger.debug -# Purposes of the following whitelist: +# Purposes of the following rule list: # * Avoiding verbose log messages when we find known non-fatal # (exitcode=1) errors # * Avoiding fatal errors when we find known crashes # * Skipping machines/devices that are known not to work out of # the box, when running in --quick mode # -# Keeping the whitelist updated is desirable, but not required, +# Keeping the rule list updated is desirable, but not required, # because unexpected cases where QEMU exits with exitcode=1 will # just trigger a INFO message. -# Valid whitelist entry keys: +# Valid error rule keys: # * accel: regexp, full match only # * machine: regexp, full match only # * device: regexp, full match only @@ -62,7 +62,7 @@ dbg = logger.debug # * expected: if True, QEMU is expected to always fail every time # when testing the corresponding test case # * loglevel: log level of log output when there's a match. -ERROR_WHITELIST = [ +ERROR_RULE_LIST = [ # Machines that won't work out of the box: # MACHINE | ERROR MESSAGE {'machine':'niagara', 'expected':True}, # Unable to load a firmware for -M niagara @@ -186,65 +186,65 @@ ERROR_WHITELIST = [ ] -def whitelistTestCaseMatch(wl, t): - """Check if a test case specification can match a whitelist entry +def errorRuleTestCaseMatch(rule, t): + """Check if a test case specification can match a error rule - This only checks if a whitelist entry is a candidate match + This only checks if a error rule is a candidate match for a given test case, it won't check if the test case - results/output match the entry. See whitelistResultMatch(). + results/output match the rule. See ruleListResultMatch(). """ - return (('machine' not in wl or + return (('machine' not in rule or 'machine' not in t or - re.match(wl['machine'] + '$', t['machine'])) and - ('accel' not in wl or + re.match(rule['machine'] + '$', t['machine'])) and + ('accel' not in rule or 'accel' not in t or - re.match(wl['accel'] + '$', t['accel'])) and - ('device' not in wl or + re.match(rule['accel'] + '$', t['accel'])) and + ('device' not in rule or 'device' not in t or - re.match(wl['device'] + '$', t['device']))) + re.match(rule['device'] + '$', t['device']))) -def whitelistCandidates(t): +def ruleListCandidates(t): """Generate the list of candidates that can match a test case""" - for i, wl in enumerate(ERROR_WHITELIST): - if whitelistTestCaseMatch(wl, t): - yield (i, wl) + for i, rule in enumerate(ERROR_RULE_LIST): + if errorRuleTestCaseMatch(rule, t): + yield (i, rule) def findExpectedResult(t): - """Check if there's an expected=True whitelist entry for a test case + """Check if there's an expected=True error rule for a test case - Returns (i, wl) tuple, where i is the index in - ERROR_WHITELIST and wl is the whitelist entry itself. + Returns (i, rule) tuple, where i is the index in + ERROR_RULE_LIST and rule is the error rule itself. """ - for i, wl in whitelistCandidates(t): - if wl.get('expected'): - return (i, wl) + for i, rule in ruleListCandidates(t): + if rule.get('expected'): + return (i, rule) -def whitelistResultMatch(wl, r): - """Check if test case results/output match a whitelist entry +def ruleListResultMatch(rule, r): + """Check if test case results/output match a error rule It is valid to call this function only if - whitelistTestCaseMatch() is True for the entry (e.g. on - entries returned by whitelistCandidates()) + errorRuleTestCaseMatch() is True for the rule (e.g. on + rules returned by ruleListCandidates()) """ - assert whitelistTestCaseMatch(wl, r['testcase']) - return ((wl.get('exitcode', 1) is None or - r['exitcode'] == wl.get('exitcode', 1)) and - ('log' not in wl or - re.search(wl['log'], r['log'], re.MULTILINE))) + assert errorRuleTestCaseMatch(rule, r['testcase']) + return ((rule.get('exitcode', 1) is None or + r['exitcode'] == rule.get('exitcode', 1)) and + ('log' not in rule or + re.search(rule['log'], r['log'], re.MULTILINE))) -def checkResultWhitelist(r): - """Look up whitelist entry for a given test case result +def checkResultRuleList(r): + """Look up error rule for a given test case result - Returns (i, wl) tuple, where i is the index in - ERROR_WHITELIST and wl is the whitelist entry itself. + Returns (i, rule) tuple, where i is the index in + ERROR_RULE_LIST and rule is the error rule itself. """ - for i, wl in whitelistCandidates(r['testcase']): - if whitelistResultMatch(wl, r): - return i, wl + for i, rule in ruleListCandidates(r['testcase']): + if ruleListResultMatch(rule, r): + return i, rule raise Exception("this should never happen") @@ -543,12 +543,12 @@ def main(): break if f: - i, wl = checkResultWhitelist(f) - dbg("testcase: %r, whitelist match: %r", t, wl) + i, rule = checkResultRuleList(f) + dbg("testcase: %r, rule list match: %r", t, rule) wl_stats.setdefault(i, []).append(f) - level = wl.get('loglevel', logging.DEBUG) + level = rule.get('loglevel', logging.DEBUG) logFailure(f, level) - if wl.get('fatal') or (args.strict and level >= logging.WARN): + if rule.get('fatal') or (args.strict and level >= logging.WARN): fatal_failures.append(f) else: dbg("success: %s", formatTestCase(t)) @@ -560,10 +560,10 @@ def main(): logger.info("Skipped %d test cases", skipped) if args.debug: - stats = sorted([(len(wl_stats.get(i, [])), wl) for i, wl in - enumerate(ERROR_WHITELIST)], key=lambda x: x[0]) - for count, wl in stats: - dbg("whitelist entry stats: %d: %r", count, wl) + stats = sorted([(len(wl_stats.get(i, [])), rule) for i, rule in + enumerate(ERROR_RULE_LIST)], key=lambda x: x[0]) + for count, rule in stats: + dbg("error rule stats: %d: %r", count, rule) if fatal_failures: for f in fatal_failures:
Replace "whitelist" in the device-crash-test script with "rule list". I'm using "rule list" instead of "allow list" or "pass list" because the list is not used only for expected/allowed errors. It also contain rules specifying which errors shouldn't be ignored and/or should be fatal. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- scripts/device-crash-test | 96 +++++++++++++++++++-------------------- 1 file changed, 48 insertions(+), 48 deletions(-)