From b1d0c4e00ed787befa9591231ac5d04d2cf6795d Mon Sep 17 00:00:00 2001 From: Cassidy Burden Date: Wed, 3 Aug 2016 09:46:20 -0700 Subject: binary search tool: Change verify_level to boolean Previously, the user could run multiple levels of verification. This would essentially repeat verification multiple times. It's unsure why a user would want to set verify_level to any value other than 0 or 1. Thus verify_level is more appropriate as a boolean. TEST=Run unit tests Change-Id: Id8832c54e2733881bf23f48af2a3b4c294117d1c Reviewed-on: https://chrome-internal-review.googlesource.com/272766 Commit-Ready: Cassidy Burden Tested-by: Cassidy Burden Reviewed-by: Caroline Tice --- binary_search_tool/README.bisect | 5 +- binary_search_tool/binary_search_state.py | 57 +++++++------ binary_search_tool/common.py | 98 ++++++++++++++-------- .../test/binary_search_tool_tester.py | 8 +- 4 files changed, 97 insertions(+), 71 deletions(-) (limited to 'binary_search_tool') diff --git a/binary_search_tool/README.bisect b/binary_search_tool/README.bisect index 19bcb74b..50df527f 100644 --- a/binary_search_tool/README.bisect +++ b/binary_search_tool/README.bisect @@ -207,6 +207,7 @@ Overriding: ./bisect.py package daisy 172.17.211.182 \ --test_script=common/hash_test.sh --install_script="" - Example 3 (enable verbose mode and disable pruning): - ./bisect.py package daisy 172.17.211.182 --verbose --noprune + Example 3 (enable verbose mode, disable pruning, and disable verification): + ./bisect.py package daisy 172.17.211.182 \ + --verbose --prune=False --verify=False diff --git a/binary_search_tool/binary_search_state.py b/binary_search_tool/binary_search_state.py index 53ba4884..13a19dca 100755 --- a/binary_search_tool/binary_search_state.py +++ b/binary_search_tool/binary_search_state.py @@ -63,7 +63,7 @@ class BinarySearchState(object): def __init__(self, get_initial_items, switch_to_good, switch_to_bad, install_script, test_script, incremental, prune, iterations, - prune_iterations, verify_level, file_args, verbose): + prune_iterations, verify, file_args, verbose): """BinarySearchState constructor, see Run for full args documentation.""" self.get_initial_items = get_initial_items self.switch_to_good = switch_to_good @@ -74,7 +74,7 @@ class BinarySearchState(object): self.prune = prune self.iterations = iterations self.prune_iterations = prune_iterations - self.verify_level = verify_level + self.verify = verify self.file_args = file_args self.verbose = verbose @@ -182,30 +182,29 @@ class BinarySearchState(object): Verify that a "good" set of items produces a "good" result and that a "bad" set of items produces a "bad" result. To be run directly before running - DoSearch. If verify_level is 0 this step is skipped. + DoSearch. If verify is False this step is skipped. """ - if not self.verify_level: + if not self.verify: return self.l.LogOutput('VERIFICATION') - self.l.LogOutput('Beginning %d tests to verify good/bad sets\n' % - self.verify_level) - for _ in range(int(self.verify_level)): - with SetFile(GOOD_SET_VAR, self.all_items), SetFile(BAD_SET_VAR, []): - self.l.LogOutput('Resetting all items to good to verify.') - self.SwitchToGood(self.all_items) - status = self.InstallScript() - assert status == 0, 'When reset_to_good, install should succeed.' - status = self.TestScript() - assert status == 0, 'When reset_to_good, status should be 0.' - - with SetFile(GOOD_SET_VAR, []), SetFile(BAD_SET_VAR, self.all_items): - self.l.LogOutput('Resetting all items to bad to verify.') - self.SwitchToBad(self.all_items) - status = self.InstallScript() - assert status == 0, 'When reset_to_bad, install should succeed.' - status = self.TestScript() - assert status == 1, 'When reset_to_bad, status should be 1.' + self.l.LogOutput('Beginning tests to verify good/bad sets\n') + + with SetFile(GOOD_SET_VAR, self.all_items), SetFile(BAD_SET_VAR, []): + self.l.LogOutput('Resetting all items to good to verify.') + self.SwitchToGood(self.all_items) + status = self.InstallScript() + assert status == 0, 'When reset_to_good, install should succeed.' + status = self.TestScript() + assert status == 0, 'When reset_to_good, status should be 0.' + + with SetFile(GOOD_SET_VAR, []), SetFile(BAD_SET_VAR, self.all_items): + self.l.LogOutput('Resetting all items to bad to verify.') + self.SwitchToBad(self.all_items) + status = self.InstallScript() + assert status == 0, 'When reset_to_bad, install should succeed.' + status = self.TestScript() + assert status == 1, 'When reset_to_bad, status should be 1.' def DoSearch(self): """Perform full search for bad items. @@ -445,7 +444,7 @@ class MockBinarySearchState(BinarySearchState): 'prune': False, 'iterations': 50, 'prune_iterations': 100, - 'verify_level': 1, + 'verify': True, 'file_args': False, 'verbose': False } @@ -468,8 +467,8 @@ def _CanonicalizeScript(script_name): def Run(get_initial_items, switch_to_good, switch_to_bad, test_script, - install_script=None, iterations=50, prune=True, noincremental=False, - file_args=False, verify_level=1, prune_iterations=100, verbose=False, + install_script=None, iterations=50, prune=False, noincremental=False, + file_args=False, verify=True, prune_iterations=100, verbose=False, resume=False): """Run binary search tool. Equivalent to running through terminal. @@ -490,8 +489,8 @@ def Run(get_initial_items, switch_to_good, switch_to_bad, test_script, noincremental: Whether to send "diffs" of good/bad items to switch scripts. file_args: If True then arguments to switch scripts will be a file name containing a newline separated list of the items to switch. - verify_level: How many verification tests to run to ensure initial good/bad - sets actually produce a good/bad result. + verify: If True, run tests to ensure initial good/bad sets actually + produce a good/bad result. prune_iterations: Max number of bad items to search for. verbose: If True will print extra debug information to user. resume: If True will resume using STATE_FILE. @@ -519,8 +518,8 @@ def Run(get_initial_items, switch_to_good, switch_to_bad, test_script, bss = BinarySearchState(get_initial_items, switch_to_good, switch_to_bad, install_script, test_script, incremental, prune, - iterations, prune_iterations, verify_level, - file_args, verbose) + iterations, prune_iterations, verify, file_args, + verbose) bss.DoVerify() try: diff --git a/binary_search_tool/common.py b/binary_search_tool/common.py index dbbb638e..f6f496ae 100644 --- a/binary_search_tool/common.py +++ b/binary_search_tool/common.py @@ -18,7 +18,6 @@ created so the help text is made properly. from __future__ import print_function -import argparse import collections import os import sys @@ -122,6 +121,15 @@ def BuildArgParser(parser, override=False): parser.add_argument(*arg_names, **arg_options) +def StrToBool(str_in): + if str_in.lower() in ['true', 't', '1']: + return True + if str_in.lower() in ['false', 'f', '0']: + return False + + raise AttributeError('%s is not a valid boolean string' % str_in) + + def _BuildArgsDict(args): """Populate ArgumentDict with all arguments""" args.AddArgument('-n', @@ -158,62 +166,80 @@ def _BuildArgsDict(args): dest='test_script', help=('Script to run to test the ' 'output after packages are built.')) + # No input (evals to False), + # --prune (evals to True), + # --prune=False, + # --prune=True args.AddArgument('-p', '--prune', dest='prune', - action='store_true', + nargs='?', + const=True, default=False, - help='Continue until all bad items are found.') - # --prune False override, opposite of --prune - args.AddArgument('--noprune', - dest='prune', - action='store_false', - default=argparse.SUPPRESS) + type=StrToBool, + metavar='bool', + help=('If True, continue until all bad items are found. ' + 'Defaults to False.')) + # No input (evals to False), + # --noincremental (evals to True), + # --noincremental=False, + # --noincremental=True args.AddArgument('-c', '--noincremental', dest='noincremental', - action='store_true', + nargs='?', + const=True, default=False, - help='Do not propagate good/bad changes incrementally.') - # --noincremental False override, opposite of --noincremental - args.AddArgument('--incremental', - dest='noincremental', - action='store_false', - default=argparse.SUPPRESS) + type=StrToBool, + metavar='bool', + help=('If True, don\'t propagate good/bad changes ' + 'incrementally. Defaults to False.')) + # No input (evals to False), + # --file_args (evals to True), + # --file_args=False, + # --file_args=True args.AddArgument('-f', '--file_args', dest='file_args', - action='store_true', + nargs='?', + const=True, default=False, - help='Use a file to pass arguments to scripts.') - # --file_args False override, opposite of --file_args - args.AddArgument('--nofile_args', - dest='file_args', - action='store_false', - default=argparse.SUPPRESS) - args.AddArgument('-v', - '--verify_level', - dest='verify_level', - type=int, - default=1, - help=('Check binary search assumptions N times ' - 'before starting.')) + type=StrToBool, + metavar='bool', + help=('Whether to use a file to pass arguments to scripts. ' + 'Defaults to False.')) + # No input (evals to True), + # --verify (evals to True), + # --verify=False, + # --verify=True + args.AddArgument('--verify', + dest='verify', + nargs='?', + const=True, + default=True, + type=StrToBool, + metavar='bool', + help=('Whether to run verify iterations before searching. ' + 'Defaults to True.')) args.AddArgument('-N', '--prune_iterations', dest='prune_iterations', type=int, help='Number of prune iterations to try in the search.', default=100) + # No input (evals to False), + # --verbose (evals to True), + # --verbose=False, + # --verbose=True args.AddArgument('-V', '--verbose', dest='verbose', - action='store_true', - help='Print full output to console.') - # --verbose False override, opposite of --verbose - args.AddArgument('--noverbose', - dest='verbose', - action='store_false', - default=argparse.SUPPRESS) + nargs='?', + const=True, + default=False, + type=StrToBool, + metavar='bool', + help='If True, print full output to console.') args.AddArgument('-r', '--resume', dest='resume', diff --git a/binary_search_tool/test/binary_search_tool_tester.py b/binary_search_tool/test/binary_search_tool_tester.py index 4ec920b8..28e6afc8 100755 --- a/binary_search_tool/test/binary_search_tool_tester.py +++ b/binary_search_tool/test/binary_search_tool_tester.py @@ -259,7 +259,7 @@ class BisectingUtilsTest(unittest.TestCase): test_script='./is_good.py', prune=True, file_args=True, - verify_level=1) + verify=True) with self.assertRaises(AssertionError): bss.DoVerify() @@ -299,7 +299,7 @@ class BisectingUtilsTest(unittest.TestCase): test_script='./is_good.py', prune=True, file_args=True, - verify_level=1) + verify=True) self.check_output() def test_noincremental_prune(self): @@ -312,7 +312,7 @@ class BisectingUtilsTest(unittest.TestCase): prune=True, noincremental=True, file_args=True, - verify_level=0) + verify=False) self.assertEquals(ret, 0) self.check_output() @@ -348,7 +348,7 @@ class BisectStressTest(unittest.TestCase): test_script='./is_good.py', prune=True, file_args=True, - verify_level=0) + verify=False) self.assertEquals(ret, 0) self.check_output() -- cgit v1.2.3