binary search tool: Fix edge case with noincremental and prune

Fix edge case where not all items were sent to switch_to_good script
when noincremental and prune were both set. Pruning progressively narrows
down the total number of items being searched across because some items
can be assumed good. This fix ensures that these "known good" items are
also sent to the switch script if noincremental is set.

TEST=Add unit test for this case, run other tests

Change-Id: I1145f6eea224f06ceb2352ec77e17e849d489606
Reviewed-on: https://chrome-internal-review.googlesource.com/270718
Commit-Ready: Cassidy Burden <cburden@google.com>
Tested-by: Cassidy Burden <cburden@google.com>
Reviewed-by: Caroline Tice <cmtice@google.com>

4 files changed, 151 insertions, 2 deletions

diff --git a/binary_search_tool/test/binary_search_tool_tester.py b/binary_search_tool/test/binary_search_tool_tester.py
index dc468e86..8488e9e0 100755
--- a/binary_search_tool/test/binary_search_tool_tester.py
+++ b/binary_search_tool/test/binary_search_tool_tester.py
@@ -123,12 +123,18 @@ class BisectingUtilsTest(unittest.TestCase):
     CleanObj()
 
     try:
-      os.remove('./installed')
       os.remove(os.readlink(binary_search_state.STATE_FILE))
-      os.remove(binary_search_state.STATE_FILE)
     except OSError:
       pass
 
+    cleanup_list = ['./installed',
+                    binary_search_state.STATE_FILE,
+                    'noinc_prune_bad',
+                    'noinc_prune_good']
+    for f in cleanup_list:
+      if os.path.exists(f):
+        os.remove(f)
+
   def runTest(self):
     ret = binary_search_state.Run(get_initial_items='./gen_init_list.py',
                                   switch_to_good='./switch_to_good.py',
@@ -296,6 +302,20 @@ class BisectingUtilsTest(unittest.TestCase):
         verify_level=1)
     self.check_output()
 
+  def test_noincremental_prune(self):
+    ret = binary_search_state.Run(
+        get_initial_items='./gen_init_list.py',
+        switch_to_good='./switch_to_good_noinc_prune.py',
+        switch_to_bad='./switch_to_bad_noinc_prune.py',
+        test_script='./is_good_noinc_prune.py',
+        install_script='./install.py',
+        prune=True,
+        noincremental=True,
+        file_args=True,
+        verify_level=0)
+    self.assertEquals(ret, 0)
+    self.check_output()
+
   def check_output(self):
     _, out, _ = command_executer.GetCommandExecuter().RunCommandWOutput(
         ('grep "Bad items are: " logs/binary_search_tool_tester.py.out | '
@@ -335,6 +355,7 @@ def Main(argv):
   suite.addTest(BisectingUtilsTest('test_early_terminate'))
   suite.addTest(BisectingUtilsTest('test_no_prune'))
   suite.addTest(BisectingUtilsTest('test_set_file'))
+  suite.addTest(BisectingUtilsTest('test_noincremental_prune'))
   suite.addTest(BisectTest('test_full_bisector'))
   runner = unittest.TextTestRunner()
   runner.run(suite)
diff --git a/binary_search_tool/test/is_good_noinc_prune.py b/binary_search_tool/test/is_good_noinc_prune.py
new file mode 100755
index 00000000..9c37db32
--- /dev/null
+++ b/binary_search_tool/test/is_good_noinc_prune.py
@@ -0,0 +1,46 @@
+#!/usr/bin/python2
+"""Check to see if the working set produces a good executable.
+
+This test script is made for the noincremental-prune test. This makes sure
+that, after pruning starts (>1 bad item is found), that the number of args sent
+to the switch scripts is equals to the actual number of items (i.e. checking
+that noincremental always holds).
+"""
+
+from __future__ import print_function
+
+import os
+import sys
+
+import common
+
+
+def Main():
+  working_set = common.ReadWorkingSet()
+
+  with open('noinc_prune_good', 'r') as good_args:
+    num_good_args = len(good_args.readlines())
+
+  with open('noinc_prune_bad', 'r') as bad_args:
+    num_bad_args = len(bad_args.readlines())
+
+  num_args = num_good_args + num_bad_args
+  if num_args != len(working_set):
+    print('Only %d args, expected %d' % (num_args, len(working_set)))
+    print('%d good args, %d bad args' % (num_good_args, num_bad_args))
+    return 3
+
+  os.remove('noinc_prune_bad')
+  os.remove('noinc_prune_good')
+
+  if not os.path.exists('./installed'):
+    return 1
+  for w in working_set:
+    if w == 1:
+      return 1  ## False, linking failure
+  return 0
+
+
+if __name__ == '__main__':
+  retval = Main()
+  sys.exit(retval)
diff --git a/binary_search_tool/test/switch_to_bad_noinc_prune.py b/binary_search_tool/test/switch_to_bad_noinc_prune.py
new file mode 100755
index 00000000..87bf1584
--- /dev/null
+++ b/binary_search_tool/test/switch_to_bad_noinc_prune.py
@@ -0,0 +1,42 @@
+#!/usr/bin/python2
+"""Switch part of the objects file in working set to (possible) bad ones.
+
+The "portion" is defined by the file (which is passed as the only argument to
+this script) content. Every line in the file is an object index, which will be
+set to good (mark as 0).
+
+This switch script is made for the noincremental-prune test. This makes sure
+that, after pruning starts (>1 bad item is found), that the number of args sent
+to the switch scripts is equals to the actual number of items (i.e. checking
+that noincremental always holds).
+
+Warning: This switch script assumes the --file_args option
+"""
+
+from __future__ import print_function
+
+import shutil
+import sys
+
+import common
+
+
+def Main(argv):
+  """Switch part of the objects file in working set to (possible) bad ones."""
+  working_set = common.ReadWorkingSet()
+  objects_file = common.ReadObjectsFile()
+  object_index = common.ReadObjectIndex(argv[1])
+
+  for oi in object_index:
+    working_set[oi] = objects_file[oi]
+
+  shutil.copy(argv[1], './noinc_prune_bad')
+
+  common.WriteWorkingSet(working_set)
+
+  return 0
+
+
+if __name__ == '__main__':
+  retval = Main(sys.argv)
+  sys.exit(retval)
diff --git a/binary_search_tool/test/switch_to_good_noinc_prune.py b/binary_search_tool/test/switch_to_good_noinc_prune.py
new file mode 100755
index 00000000..c5e78e45
--- /dev/null
+++ b/binary_search_tool/test/switch_to_good_noinc_prune.py
@@ -0,0 +1,40 @@
+#!/usr/bin/python2
+"""Change portions of the object files to good.
+
+The "portion" is defined by the file (which is passed as the only argument to
+this script) content. Every line in the file is an object index, which will be
+set to good (mark as 0).
+
+This switch script is made for the noincremental-prune test. This makes sure
+that, after pruning starts (>1 bad item is found), that the number of args sent
+to the switch scripts is equals to the actual number of items (i.e. checking
+that noincremental always holds).
+
+Warning: This switch script assumes the --file_args option
+"""
+
+from __future__ import print_function
+
+import shutil
+import sys
+
+import common
+
+
+def Main(argv):
+  working_set = common.ReadWorkingSet()
+  object_index = common.ReadObjectIndex(argv[1])
+
+  for oi in object_index:
+    working_set[int(oi)] = 0
+
+  shutil.copy(argv[1], './noinc_prune_good')
+
+  common.WriteWorkingSet(working_set)
+
+  return 0
+
+
+if __name__ == '__main__':
+  retval = Main(sys.argv)
+  sys.exit(retval)