Pass the cluster mode arguments through to pybnf in the full test runner

tests/full_tests/run_all.py

@@ -8,34 +8,42 @@
 Run with no arguments to run locally
 Run with the argument ssh to use PyBNF's automatic cluster setup with dask ssh
 Run with the argument sf to use manual cluster setup, with a preconfigured dask cluster with scheduler file named sf
-Bash files are provided in this folder to run this script using the cluster options. 
+Bash files are provided in this folder to run this script using the cluster options.
 
 This test script is not Windows compatible due to extensive use of `grep`
 
-The output file test_summary.txt should be manually checked to confirm success of the tests. 
-The output can be compared to example_summary.txt, which is the result from running on commit e34ba3b (14 Mar 2019)
-shortly before the v1.0.0 release.
+The output file test_summary.txt should be checked by hand to confirm the tests worked.
+Every test should say Completed, with a low objective value. For T5 and T7, the credible
+intervals should bracket the ground truth values noted next to them.
+
+For comparison, this folder holds reference output from a recent run: example_summary.txt
+for the local mode, example_summary_ssh.txt for the ssh mode, and example_summary_sf.txt
+for the scheduler file mode. Fitting starts from random points, so the exact objective
+values and run times will differ from one run to the next. Use the reference output as a
+rough guide, not an exact target.
 """
 
 import subprocess as sp
 from sys import argv
 import traceback as tb
 
-def run_all():
+def run_all(output_name, extra_args):
     """
-    Run all test cases and write summary to test_summary.txt
+    Run all test cases and write the summary to output_name.
+    extra_args is a list of extra command line arguments passed to pybnf for
+    every test, used to select the cluster mode.
     """
 
     outstream = open(output_name,'w')
-    
+
     try:
-        run_test('T1','T1-ssprop','polynomial.conf',outstream)
+        run_test('T1','T1-ssprop','polynomial.conf',outstream,extra_args)
     except Exception:
         outstream.write('Error on T1!\n')
         outstream.write(tb.format_exc())
 
     try:
-        run_test('T2','T2-ade-abcd','fit_ade.conf',outstream)
+        run_test('T2','T2-ade-abcd','fit_ade.conf',outstream,extra_args)
         # Also check refined fit
         bestname, bestvalue = read_best_fit('T2-ade-abcd/fit/Results/sorted_params_refine_final.txt')
         outstream.write('After refinement, best objective was %s with param set %s\n' % (bestvalue, bestname))
@@ -44,7 +52,7 @@ def run_all():
         outstream.write(tb.format_exc())
 
     try:
-        run_test('T3','T3-de-egg','egg-de.conf',outstream)
+        run_test('T3','T3-de-egg','egg-de.conf',outstream,extra_args)
         # Also check bootstrap results
         bestname, bestvalue = read_best_fit('T3-de-egg/fit/Results-boot0/sorted_params_final.txt')
         outstream.write('Bootstrap 0 best objective was %s with param set %s\n' % (bestvalue, bestname))
@@ -57,13 +65,13 @@ def run_all():
         outstream.write(tb.format_exc())
 
     try:
-        run_test('T4','T4-pso-nf','rnf.conf',outstream)
+        run_test('T4','T4-pso-nf','rnf.conf',outstream,extra_args)
     except Exception:
         outstream.write('Error on T4!\n')
         outstream.write(tb.format_exc())
 
     try:
-        run_test('T5','T5-pt-trivial','trivial_pt.conf',outstream)
+        run_test('T5','T5-pt-trivial','trivial_pt.conf',outstream,extra_args)
         # Also check credible intervals
         with open('T5-pt-trivial/fit/Results/credible68_final.txt') as f:
             f.readline() # header
@@ -86,7 +94,7 @@ def run_all():
         outstream.write(tb.format_exc())
 
     try:
-        run_test('T6','T6-check','polynomial.conf',outstream, display_best=False)
+        run_test('T6','T6-check','polynomial.conf',outstream,extra_args, display_best=False)
         try:
             grep = sp.run(['grep', 'Objective', 'T6_stdout.out'],
                       check=True, stdout=sp.PIPE, universal_newlines=True)
@@ -106,7 +114,7 @@ def run_all():
         outstream.write(tb.format_exc())
 
     try:
-        run_test('T7','T7-dream-trivial','trivial_dream.conf',outstream)
+        run_test('T7','T7-dream-trivial','trivial_dream.conf',outstream,extra_args)
         # Check credible intervals - same ground truth as T5
         with open('T7-dream-trivial/fit/Results/credible68_final.txt') as f:
             f.readline() # header
@@ -128,15 +136,17 @@ def run_all():
         outstream.write('Error on T7!\n')
         outstream.write(tb.format_exc())
 
-def run_test(name, folder, conffile, outstream, display_best=True):
+def run_test(name, folder, conffile, outstream, extra_args, display_best=True):
     """
-    Run the specified test case, writing summary to outstream
+    Run the specified test case, writing summary to outstream.
+    extra_args is a list of extra command line arguments passed to pybnf, used to
+    select the cluster mode.
     """
-    
+
     outstream.write('\nTest %s:\n' % name)
-
 
-    proc = sp.run(['pybnf', '-c', '%s/%s' % (folder, conffile), '-l', name, '-o'],
+
+    proc = sp.run(build_pybnf_command(folder, conffile, name, extra_args),
                   stdout=sp.PIPE, stderr=sp.PIPE, universal_newlines=True)
     with open('%s_stdout.out' % name, 'w') as out:
         out.write(proc.stdout)
@@ -192,16 +202,31 @@ def read_best_fit(path):
     return (name, value)
 
 
-if __name__=='__main__':
-    if len(argv) == 1:
-        extra_args = []
-        output_name = 'test_summary.txt'
-    elif argv[1] == 'ssh':
-        extra_args = ['-t', 'slurm']
-        output_name = 'test_summary_ssh.txt'
-    elif argv[1] == 'sf':
-        extra_args = ['-s', 'sf']
-        output_name = 'test_summary_sf.txt'
+def build_pybnf_command(folder, conffile, name, extra_args):
+    """
+    Return the pybnf command line for one test case, as a list of strings.
+    extra_args, which selects the cluster mode, is added to the end so it reaches pybnf.
+    """
+    return ['pybnf', '-c', '%s/%s' % (folder, conffile), '-l', name, '-o'] + list(extra_args)
+
+
+def parse_mode(args):
+    """
+    Turn the command line arguments (everything after the script name) into the pair
+    (extra_args, output_name). With no arguments the tests run locally. The argument
+    ssh selects PyBNF's dask ssh cluster setup. The argument sf selects manual cluster
+    setup with a scheduler file named sf. Any other argument is an error.
+    """
+    if len(args) == 0:
+        return [], 'test_summary.txt'
+    elif args[0] == 'ssh':
+        return ['-t', 'slurm'], 'test_summary_ssh.txt'
+    elif args[0] == 'sf':
+        return ['-s', 'sf'], 'test_summary_sf.txt'
     else:
-        raise ValueError('Invalid argument '+argv[1])
-    run_all()
+        raise ValueError('Invalid argument ' + args[0])
+
+
+if __name__=='__main__':
+    extra_args, output_name = parse_mode(argv[1:])
+    run_all(output_name, extra_args)

tests/test_full_test_runner.py

@@ -0,0 +1,66 @@
+"""
+Tests for the helper functions in tests/full_tests/run_all.py.
+
+run_all.py is a standalone script rather than part of the package, so it is loaded
+here by path. The script guards its real work behind an if __name__ == '__main__'
+block, so importing it has no side effects.
+"""
+
+import importlib.util
+import os
+
+import pytest
+
+RUN_ALL_PATH = os.path.join(os.path.dirname(__file__), 'full_tests', 'run_all.py')
+
+
+def load_run_all():
+    spec = importlib.util.spec_from_file_location('full_tests_run_all', RUN_ALL_PATH)
+    module = importlib.util.module_from_spec(spec)
+    spec.loader.exec_module(module)
+    return module
+
+
+run_all = load_run_all()
+
+
+def test_parse_mode_local_when_no_arguments():
+    extra_args, output_name = run_all.parse_mode([])
+    assert extra_args == []
+    assert output_name == 'test_summary.txt'
+
+
+def test_parse_mode_ssh_selects_the_slurm_cluster():
+    extra_args, output_name = run_all.parse_mode(['ssh'])
+    assert extra_args == ['-t', 'slurm']
+    assert output_name == 'test_summary_ssh.txt'
+
+
+def test_parse_mode_sf_selects_the_scheduler_file():
+    extra_args, output_name = run_all.parse_mode(['sf'])
+    assert extra_args == ['-s', 'sf']
+    assert output_name == 'test_summary_sf.txt'
+
+
+def test_parse_mode_rejects_an_unknown_argument():
+    with pytest.raises(ValueError):
+        run_all.parse_mode(['nonsense'])
+
+
+def test_build_pybnf_command_passes_the_cluster_arguments_through():
+    # This guards the bug where the cluster arguments were worked out but never
+    # reached pybnf, so the ssh and sf modes quietly ran on a single machine.
+    command = run_all.build_pybnf_command(
+        'T1-ssprop', 'polynomial.conf', 'T1', ['-t', 'slurm']
+    )
+    assert command[0] == 'pybnf'
+    assert '-c' in command
+    assert 'T1-ssprop/polynomial.conf' in command
+    assert command[command.index('-l') + 1] == 'T1'
+    assert '-o' in command
+    assert command[-2:] == ['-t', 'slurm']
+
+
+def test_build_pybnf_command_has_no_extra_arguments_in_local_mode():
+    command = run_all.build_pybnf_command('T1-ssprop', 'polynomial.conf', 'T1', [])
+    assert command == ['pybnf', '-c', 'T1-ssprop/polynomial.conf', '-l', 'T1', '-o']