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 <cburden@google.com>
Tested-by: Cassidy Burden <cburden@google.com>
Reviewed-by: Caroline Tice <cmtice@google.com>

4 files changed, 97 insertions, 71 deletions

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()