Merge pull request #786 from qwhelan/timeout_bisection

Allow asv find to perform a bisection even if benchmark times out

Author: pv

CHANGES.rst

@@ -13,6 +13,9 @@ API Changes
 
 Bug Fixes
 ^^^^^^^^^
+- When an ``asv find`` step fails due to timeout, assume runtime equal to
+  timeout to allow bisection to proceed (#768)
+
 
 Other Changes and Additions
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^

asv/commands/find.py

@@ -111,6 +111,7 @@ def run(cls, conf, range_spec, bench, invert=False, show_stderr=False, parallel=
                 return 1
 
         benchmark_name, = benchmarks.keys()
+        benchmark_type = benchmarks[benchmark_name]["type"]
 
         steps = int(math.log(len(commit_hashes)) / math.log(2))
 
@@ -144,6 +145,15 @@ def do_benchmark(i):
 
             results[i] = result
 
+            # If we failed due to timeout in a timing benchmark, set
+            # runtime as the timeout to prevent falling back to linear
+            # search
+            errcode = res.errcode[benchmark_name]
+            if errcode == util.TIMEOUT_RETCODE and benchmark_type == "time":
+                timeout_limit = benchmarks[benchmark_name]['timeout']
+                results[i] = [r if r is not None else timeout_limit
+                              for r in results[i]]
+
             return results[i]
 
         def non_null_results(*results):

test/benchmark/params_examples.py

@@ -94,6 +94,17 @@ def track_find_test(n):
 track_find_test.params = [1, 2]
 
 
+def time_find_test_timeout():
+    import asv_test_repo, time
+    if asv_test_repo.dummy_value[1] < 0:
+        time.sleep(100)
+
+time_find_test_timeout.timeout = 1.0
+time_find_test_timeout.repeat = 1
+time_find_test_timeout.number = 1
+time_find_test_timeout.warmup_time = 0
+
+
 def track_param_selection(a, b):
     return a + b
 

test/test_benchmarks.py

@@ -73,7 +73,7 @@ def test_discover_benchmarks(benchmarks_fixture):
     b = benchmarks.Benchmarks.discover(conf, repo, envs, [commit_hash],
                                        regex='example')
     conf.branches = old_branches
-    assert len(b) == 35
+    assert len(b) == 36
 
     b = benchmarks.Benchmarks.discover(conf, repo, envs, [commit_hash],
                               regex='time_example_benchmark_1')
@@ -106,7 +106,7 @@ def test_discover_benchmarks(benchmarks_fixture):
     assert b._benchmark_selection['params_examples.track_param_selection'] == [0, 1, 2, 3]
 
     b = benchmarks.Benchmarks.discover(conf, repo, envs, [commit_hash])
-    assert len(b) == 49
+    assert len(b) == 50
 
     assert 'named.OtherSuite.track_some_func' in b
 

test/test_find.py

@@ -12,21 +12,27 @@
 from asv.util import check_output, which
 
 from . import tools
-from .tools import dummy_packages
-from .test_workflow import basic_conf
+from .test_workflow import generate_basic_conf
 
 
 WIN = (os.name == 'nt')
 
 
-def test_find(capfd, basic_conf):
-    tmpdir, local, conf, machine_file = basic_conf
+def test_find(capfd, tmpdir):
+    values = [
+        (None, None),
+        (1, 1),
+        (3, 1),
+        (None, 1),
+        (6, None),
+        (5, 1),
+        (6, 1),
+        (6, 1),
+        (6, 6),
+        (6, 6),
+    ]
 
-    if WIN and os.path.basename(sys.argv[0]).lower().startswith('py.test'):
-        # Multiprocessing in spawn mode can result to problems with py.test
-        # Find.run calls Setup.run in parallel mode by default
-        pytest.skip("Multiprocessing spawn mode on Windows not safe to run "
-                    "from py.test runner.")
+    tmpdir, local, conf, machine_file = generate_basic_conf(tmpdir, values=values, dummy_packages=False)
 
     # Test find at least runs
     tools.run_asv_with_conf(conf, 'find', "master~5..master", "params_examples.track_find_test",
@@ -39,3 +45,27 @@ def test_find(capfd, basic_conf):
         [which('git'), 'rev-parse', 'master^'], cwd=conf.repo)
 
     assert "Greatest regression found: {0}".format(regression_hash[:8]) in output
+
+
+@pytest.mark.flaky(reruns=1, reruns_delay=5)  # depends on a timeout
+def test_find_timeout(capfd, tmpdir):
+    values = [
+        (1, 0),
+        (1, 0),
+        (1, -1)
+    ]
+
+    tmpdir, local, conf, machine_file = generate_basic_conf(tmpdir, values=values, dummy_packages=False)
+
+    # Test find at least runs
+    tools.run_asv_with_conf(conf, 'find', "-e", "master", "params_examples.time_find_test_timeout",
+                            _machine_file=machine_file)
+
+    # Check it found the first commit after the initially tested one
+    output, err = capfd.readouterr()
+
+    regression_hash = check_output(
+        [which('git'), 'rev-parse', 'master'], cwd=conf.repo)
+
+    assert "Greatest regression found: {0}".format(regression_hash[:8]) in output
+    assert "asv: benchmark timed out (timeout 1.0s)" in output

test/test_run.py

@@ -4,6 +4,7 @@
 from __future__ import (absolute_import, division, print_function,
                         unicode_literals)
 
+import sys
 import os
 from os.path import join
 
@@ -20,7 +21,7 @@
 from asv.commands.run import Run
 
 from . import tools
-from .tools import dummy_packages
+from .tools import dummy_packages, WIN
 from .test_workflow import basic_conf, generate_basic_conf
 
 
@@ -323,6 +324,12 @@ def check_env_matrix(env_build, env_nobuild):
 def test_parallel(basic_conf, dummy_packages):
     tmpdir, local, conf, machine_file = basic_conf
 
+    if WIN and os.path.basename(sys.argv[0]).lower().startswith('py.test'):
+        # Multiprocessing in spawn mode can result to problems with py.test
+        # Find.run calls Setup.run in parallel mode by default
+        pytest.skip("Multiprocessing spawn mode on Windows not safe to run "
+                    "from py.test runner.")
+
     conf.matrix = {
         "req": dict(conf.matrix),
         "env": {"SOME_TEST_VAR": ["1", "2"]},

test/test_workflow.py

@@ -25,21 +25,13 @@
 from .tools import dummy_packages, get_default_environment_type
 
 
-dummy_values = [
-    (None, None),
-    (1, 1),
-    (3, 1),
-    (None, 1),
-    (6, None),
-    (5, 1),
-    (6, 1),
+dummy_values = (
     (6, 1),
     (6, 6),
     (6, 6),
-]
-
+)
 
-def generate_basic_conf(tmpdir, repo_subdir=''):
+def generate_basic_conf(tmpdir, repo_subdir='', values=dummy_values, dummy_packages=True):
     tmpdir = six.text_type(tmpdir)
     local = abspath(dirname(__file__))
     os.chdir(tmpdir)
@@ -54,9 +46,17 @@ def generate_basic_conf(tmpdir, repo_subdir=''):
     shutil.copyfile(join(local, 'asv-machine.json'),
                     machine_file)
 
-    repo_path = tools.generate_test_repo(tmpdir, dummy_values,
+    repo_path = tools.generate_test_repo(tmpdir, values,
                                          subdir=repo_subdir).path
 
+    if dummy_packages:
+        matrix = {
+            "asv_dummy_test_package_1": [""],
+            "asv_dummy_test_package_2": tools.DUMMY2_VERSIONS,
+        }
+    else:
+        matrix = {}
+
     conf_dict = {
         'env_dir': 'env',
         'benchmark_dir': 'benchmark',
@@ -65,10 +65,7 @@ def generate_basic_conf(tmpdir, repo_subdir=''):
         'repo': relpath(repo_path),
         'dvcs': 'git',
         'project': 'asv',
-        'matrix': {
-            "asv_dummy_test_package_1": [""],
-            "asv_dummy_test_package_2": tools.DUMMY2_VERSIONS,
-        },
+        'matrix': matrix,
     }
     if repo_subdir:
         conf_dict['repo_subdir'] = repo_subdir
@@ -96,7 +93,7 @@ def test_run_publish(capfd, basic_conf):
     }
 
     # Tests a typical complete run/publish workflow
-    tools.run_asv_with_conf(conf, 'run', "master~5..master", '--steps=2',
+    tools.run_asv_with_conf(conf, 'run', "master", '--steps=2',
                             '--quick', '--show-stderr', '--profile',
                             '-a', 'warmup_time=0',
                             '--durations=5',
@@ -137,12 +134,12 @@ def test_run_publish(capfd, basic_conf):
 
     # Check that the skip options work
     capfd.readouterr()
-    tools.run_asv_with_conf(conf, 'run', "master~5..master", '--steps=2',
+    tools.run_asv_with_conf(conf, 'run', "master", '--steps=2',
                             '--quick', '--skip-existing-successful',
                             '--bench=time_secondary.track_value',
                             '--skip-existing-failed',
                             _machine_file=join(tmpdir, 'asv-machine.json'))
-    tools.run_asv_with_conf(conf, 'run', "master~5..master", '--steps=2',
+    tools.run_asv_with_conf(conf, 'run', "master", '--steps=2',
                             '--bench=time_secondary.track_value',
                             '--quick', '--skip-existing-commits',
                             _machine_file=join(tmpdir, 'asv-machine.json'))