zephyr: zmake: remove fail_fast functionality

This functionality is something which fundamentally leads to buggy
code, as allowing multiproc.Executor.wait() to return while jobs are
actually still running means the programmer needs to consider the case
that subprocesses are still doing work.

Remove it.

BUG=chromium:1188822,b:182818881,b:185163907
BRANCH=none
TEST=zmake testall

Signed-off-by: Jack Rosenthal <jrosenth@chromium.org>
Change-Id: Iffa9770734ce4fce8d62de2e3472235eb1cf9c4c
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2821606
Reviewed-by: Jeremy Bettis <jbettis@chromium.org>

4 files changed, 14 insertions, 55 deletions

diff --git a/zephyr/zmake/tests/test_multiproc_executor.py b/zephyr/zmake/tests/test_multiproc_executor.py
index e67d3c0904..46aafbb823 100644
--- a/zephyr/zmake/tests/test_multiproc_executor.py
+++ b/zephyr/zmake/tests/test_multiproc_executor.py
@@ -7,31 +7,19 @@ import zmake.multiproc
 
 
 def test_single_function_executor_success():
-    executor = zmake.multiproc.Executor(fail_fast=True)
-    executor.append(lambda: 0)
-    assert executor.wait() == 0
-
-    executor = zmake.multiproc.Executor(fail_fast=False)
+    executor = zmake.multiproc.Executor()
     executor.append(lambda: 0)
     assert executor.wait() == 0
 
 
 def test_single_function_executor_fail():
-    executor = zmake.multiproc.Executor(fail_fast=True)
-    executor.append(lambda: -1)
-    assert executor.wait() == -1
-
-    executor = zmake.multiproc.Executor(fail_fast=False)
+    executor = zmake.multiproc.Executor()
     executor.append(lambda: -2)
     assert executor.wait() == -2
 
 
 def test_single_function_executor_raise():
-    executor = zmake.multiproc.Executor(fail_fast=True)
-    executor.append(lambda: 1/0)
-    assert executor.wait() != 0
-
-    executor = zmake.multiproc.Executor(fail_fast=False)
+    executor = zmake.multiproc.Executor()
     executor.append(lambda: 1/0)
     assert executor.wait() != 0
 
@@ -47,14 +35,7 @@ def _lock_step(cv, predicate, step, return_value=0):
 def test_two_function_executor_wait_for_both():
     cv = threading.Condition()
     step = [0]
-    executor = zmake.multiproc.Executor(fail_fast=True)
-    executor.append(lambda: _lock_step(cv=cv, predicate=0, step=step))
-    executor.append(lambda: _lock_step(cv=cv, predicate=1, step=step))
-    assert executor.wait() == 0
-    assert step[0] == 2
-
-    step = [0]
-    executor = zmake.multiproc.Executor(fail_fast=False)
+    executor = zmake.multiproc.Executor()
     executor.append(lambda: _lock_step(cv=cv, predicate=0, step=step))
     executor.append(lambda: _lock_step(cv=cv, predicate=1, step=step))
     assert executor.wait() == 0
@@ -64,14 +45,7 @@ def test_two_function_executor_wait_for_both():
 def test_two_function_executor_one_fails():
     cv = threading.Condition()
     step = [0]
-    executor = zmake.multiproc.Executor(fail_fast=True)
-    executor.append(lambda: _lock_step(cv=cv, predicate=0, step=step, return_value=-1))
-    executor.append(lambda: _lock_step(cv=cv, predicate=2, step=step))
-    assert executor.wait() == -1
-    assert step[0] == 1
-
-    step = [0]
-    executor = zmake.multiproc.Executor(fail_fast=False)
+    executor = zmake.multiproc.Executor()
     executor.append(lambda: _lock_step(cv=cv, predicate=0, step=step, return_value=-1))
     executor.append(lambda: _lock_step(cv=cv, predicate=1, step=step))
     assert executor.wait() == -1
diff --git a/zephyr/zmake/zmake/__main__.py b/zephyr/zmake/zmake/__main__.py
index 8dfc57eede..78a9cf4d65 100644
--- a/zephyr/zmake/zmake/__main__.py
+++ b/zephyr/zmake/zmake/__main__.py
@@ -114,12 +114,8 @@ def main(argv=None):
                       help='The build directory used during configuration')
 
     testall = sub.add_parser('testall')
-    testall.add_argument('--fail-fast', action='store_true',
-                         help='stop testing after the first error')
 
     coverage = sub.add_parser('coverage')
-    coverage.add_argument('--fail-fast', action='store_true',
-                         help='stop testing after the first error')
     coverage.add_argument('build_dir', type=pathlib.Path,
                       help='The build directory used during configuration')
 
diff --git a/zephyr/zmake/zmake/multiproc.py b/zephyr/zmake/zmake/multiproc.py
index 966d90c567..54df471652 100644
--- a/zephyr/zmake/zmake/multiproc.py
+++ b/zephyr/zmake/zmake/multiproc.py
@@ -203,22 +203,16 @@ class Executor:
 
     This class is used to run multiple functions in parallel. The functions MUST
     return an integer result code (or throw an exception). This class will start
-    a thread per operation and wait() for all the threads to resolve. If
-    fail_fast is set to True, then not all threads must return before wait()
-    returns. Instead either ALL threads must return 0 OR any thread must return
-    a non zero result (or throw an exception).
+    a thread per operation and wait() for all the threads to resolve.
 
     Attributes:
-        fail_fast: Whether or not the first function's error code should
-         terminate the executor.
         lock: The condition variable used to synchronize across threads.
         threads: A list of threading.Thread objects currently under this
          Executor.
         results: A list of result codes returned by each of the functions called
          by this Executor.
     """
-    def __init__(self, fail_fast):
-        self.fail_fast = fail_fast
+    def __init__(self):
         self.lock = threading.Condition()
         self.threads = []
         self.results = []
@@ -247,13 +241,8 @@ class Executor:
     def wait(self):
         """Wait for a result to be available.
 
-        This function waits for the executor to resolve. Being resolved depends on
-        the initial fail_fast setting.
-        - If fail_fast is True then the executor is resolved as soon as any thread
-          throws an exception or returns a non-zero result. Or, all the threads
-          returned a zero result code.
-        - If fail_fast is False, then all the threads must have returned a result
-          code or have thrown.
+        This function waits for the executor to resolve (i.e., all
+        threads have finished).
 
         Returns:
             An integer result code of either the first failed function or 0 if
@@ -292,7 +281,7 @@ class Executor:
         """
         if len(self.threads) == len(self.results):
             return True
-        return self.fail_fast and any([result for result in self.results])
+        return False
 
     @property
     def _result(self):
diff --git a/zephyr/zmake/zmake/zmake.py b/zephyr/zmake/zmake/zmake.py
index f341de3030..ed6087a718 100644
--- a/zephyr/zmake/zmake/zmake.py
+++ b/zephyr/zmake/zmake/zmake.py
@@ -417,9 +417,9 @@ class Zmake:
         for test_file in directory.glob('test_*.py'):
             executor.append(func=lambda: run_test(test_file))
 
-    def testall(self, fail_fast=False):
+    def testall(self):
         """Test all the valid test targets"""
-        executor = zmake.multiproc.Executor(fail_fast=fail_fast)
+        executor = zmake.multiproc.Executor()
         tmp_dirs = []
         for project in zmake.project.find_projects(
                 self.module_paths['ec'] / 'zephyr'):
@@ -536,9 +536,9 @@ class Zmake:
             return rv
         return self._run_lcov(build_dir, lcov_file, initial=False)
 
-    def coverage(self, build_dir, fail_fast=False):
+    def coverage(self, build_dir):
         """Builds all targets with coverage enabled, and then runs the tests."""
-        executor = zmake.multiproc.Executor(fail_fast=fail_fast)
+        executor = zmake.multiproc.Executor()
         all_lcov_files = []
         root_dir = self.module_paths['ec'] / 'zephyr'
         for project in zmake.project.find_projects(root_dir):