buptest: base testing subproc funcs ex() and exo() on run()

Rework the subprocess functions, providing ex() and exo() as concise
test functions that print the commands they're executing.  Base them
on a common, lower-level run() function.

Drop exc() since ex() and exo() both check the exit status by default.

Signed-off-by: Rob Browning <rlb@defaultvalue.org>
Tested-by: Rob Browning <rlb@defaultvalue.org>

diff --git a/buptest.py b/buptest.py
index c80d1ef70dc2f961328e377e4442c88643f2f320..dd145eed2d467b0170396849f04f062cebf7b180 100644 (file)
--- a/buptest.py
+++ b/buptest.py
@@ -4,7 +4,7 @@ from collections import namedtuple
 from contextlib import contextmanager
 from os.path import basename, dirname, realpath
 from pipes import quote
-from subprocess import PIPE, Popen, check_call
+from subprocess import PIPE, Popen
 from traceback import extract_stack
 import errno, os, subprocess, sys, tempfile
 
@@ -51,41 +51,49 @@ def test_tempdir(prefix):
         subprocess.call(['rm', '-rf', tmpdir])
 
 
+ex_res = namedtuple('SubprocResult', ['out', 'err', 'proc', 'rc'])
+
+def run(cmd, check=True, input=None, **kwargs):
+    """Run a subprocess as per subprocess.Popen(cmd, **kwargs) followed by
+    communicate(input=input).  If check is true, then throw an
+    exception if the subprocess exits with non-zero status.  Return a
+    SubprocResult tuple.
+
+    """
+    if input:
+        assert 'stdin' not in kwargs
+        kwargs['stdin'] = PIPE
+    p = Popen(cmd, **kwargs)
+    out, err = p.communicate(input=input)
+    if check and p.returncode != 0:
+        raise Exception('subprocess %r failed with status %d%s'
+                        % (' '.join(map(quote, cmd)), p.returncode,
+                           (', stderr: %r' % err) if err else ''))
+    return ex_res(out=out, err=err, proc=p, rc=p.returncode)
+
 def logcmd(cmd):
     if isinstance(cmd, basestring):
         print(cmd, file=sys.stderr)
     else:
         print(' '.join(map(quote, cmd)), file=sys.stderr)
 
-
-SubprocInfo = namedtuple('SubprocInfo', ('out', 'err', 'rc', 'p'))
-
-def exo(cmd, input=None, stdin=None, stdout=PIPE, stderr=PIPE,
-        shell=False, check=True):
-    """Print cmd to stderr, run it, and return the resulting SubprocInfo.
-    The keyword arguments are passed to Popen, and the defaults
-    capture both stdout and stderr.
-
+def ex(cmd, **kwargs):
+    """Print cmd to stderr and then run it as per ex(...).
+    Print the subprocess stderr to stderr if stderr=PIPE and there's
+    any data.
     """
     logcmd(cmd)
-    p = Popen(cmd,
-              stdin=(PIPE if input else stdin),
-              stdout=stdout,
-              stderr=stderr,
-              shell=shell)
-    out, err = p.communicate(input=input)
-    if check and p.returncode != 0:
-        raise Exception('subprocess %r failed with status %d%s'
-                        % (' '.join(map(quote, cmd)),
-                           p.returncode,
-                           (', stderr: %r' % err) if stderr else ''))
-    return SubprocInfo(out=out, err=err, rc=p.returncode, p=p)
+    result = run(cmd, **kwargs)
+    if result.err:
+        sys.stderr.write(result.err)
+    return result
 
-def exc(cmd, input=None, stdout=None, stderr=None, shell=False, check=True):
-    """Print cmd to stderr, run it, and return the resulting SubprocInfo.
-    The keyword arguments are passed to Popen, and the defaults
-    allow stdout and stderr to pass through.
+def exo(cmd, **kwargs):
+    """Print cmd to stderr and then run it as per ex(..., stdout=PIPE).
+    Print the subprocess stderr to stderr if stderr=PIPE and there's
+    any data.
 
     """
-    return exo(cmd, input=input, stdout=stdout, stderr=stderr, shell=shell,
-               check=check)
+    assert 'stdout' not in kwargs
+    kwargs['stdout'] = PIPE
+    return ex(cmd, **kwargs)

diff --git a/t/test-ftp b/t/test-ftp
index 9431f14f6ad826d7fa1f3c4a473afc3045acb048..c71bc54be79f512560b1a6d416bfc2d145e3e8b1 100755 (executable)
--- a/t/test-ftp
+++ b/t/test-ftp
@@ -8,6 +8,7 @@ exec "$bup_python" "$0" ${1+"$@"}
 from __future__ import absolute_import, print_function
 from os import environ, chdir, mkdir, symlink, unlink
 from os.path import abspath, dirname
+from subprocess import PIPE
 from time import localtime, strftime
 import os, sys
 
@@ -16,16 +17,15 @@ sys.path[:0] = [abspath(script_home + '/../lib'), abspath(script_home + '/..')]
 top = os.getcwd()
 bup_cmd = top + '/bup'
 
-from buptest import exc, exo, logcmd, test_tempdir
+from buptest import ex, exo, logcmd, test_tempdir
 from wvtest import wvfail, wvpass, wvpasseq, wvpassne, wvstart
 
 from bup.helpers import unlink as unlink_if_exists
 
 def bup(*args, **kwargs):
-    return exo((bup_cmd,) + args, **kwargs)[0]
-
-def bupc(*args, **kwargs):
-    return exc((bup_cmd,) + args, **kwargs)
+    if 'stdout' not in kwargs:
+        return exo((bup_cmd,) + args, **kwargs)
+    return ex((bup_cmd,) + args, **kwargs)
 
 def jl(*lines):
     return ''.join(line + '\n' for line in lines)
@@ -52,80 +52,82 @@ with test_tempdir('ftp-') as tmpdir:
     symlink('not-there', 'bad-symlink')
 
     chdir(tmpdir)    
-    bupc('init')
-    bupc('index', 'src')
-    bupc('save', '-n', 'src', '--strip', 'src')
+    bup('init')
+    bup('index', 'src')
+    bup('save', '-n', 'src', '--strip', 'src')
     save_utc = int(exo(('git', 'show', '-s', '--format=%at', 'src')).out.strip())
     save_name = strftime('%Y-%m-%d-%H%M%S', localtime(save_utc))
     
     wvstart('help')
     wvpasseq('Commands: ls cd pwd cat get mget help quit\n',
-             exo((bup_cmd, 'ftp'), input='help\n').err)
+             exo((bup_cmd, 'ftp'), input='help\n', stderr=PIPE).err)
 
     wvstart('pwd/cd')
-    wvpasseq('/\n', bup('ftp', input='pwd\n'))
-    wvpasseq('', bup('ftp', input='cd src\n'))
-    wvpasseq('/src\n', bup('ftp', input=jl('cd src', 'pwd')))
-    wvpasseq('/src\n/\n', bup('ftp', input=jl('cd src', 'pwd', 'cd ..', 'pwd')))
+    wvpasseq('/\n', bup('ftp', input='pwd\n').out)
+    wvpasseq('', bup('ftp', input='cd src\n').out)
+    wvpasseq('/src\n', bup('ftp', input=jl('cd src', 'pwd')).out)
+    wvpasseq('/src\n/\n', bup('ftp', input=jl('cd src', 'pwd',
+                                              'cd ..', 'pwd')).out)
     wvpasseq('/src\n/\n', bup('ftp', input=jl('cd src', 'pwd',
-                                              'cd ..', 'cd ..', 'pwd')))
+                                              'cd ..', 'cd ..', 'pwd')).out)
     wvpasseq('/src/latest/dir\n',
-             bup('ftp', input=jl('cd src/latest/dir-symlink', 'pwd')))
+             bup('ftp', input=jl('cd src/latest/dir-symlink', 'pwd')).out)
     wvpasseq('/src/latest/dir\n',
-             bup('ftp', input=jl('cd src latest dir-symlink', 'pwd')))
-    wvpassne(0, bupc('ftp',
-                     input=jl('cd src/latest/bad-symlink', 'pwd'),
-                     check=False).rc)
-    wvpassne(0, bupc('ftp',
-                     input=jl('cd src/latest/not-there', 'pwd'),
-                     check=False).rc)
+             bup('ftp', input=jl('cd src latest dir-symlink', 'pwd')).out)
+    wvpassne(0, bup('ftp',
+                    input=jl('cd src/latest/bad-symlink', 'pwd'),
+                    check=False, stdout=None).rc)
+    wvpassne(0, bup('ftp',
+                    input=jl('cd src/latest/not-there', 'pwd'),
+                    check=False, stdout=None).rc)
 
     wvstart('ls')
     # FIXME: elaborate
-    wvpasseq('src\n', bup('ftp', input='ls\n'))
+    wvpasseq('src\n', bup('ftp', input='ls\n').out)
     wvpasseq(save_name + '\nlatest\n',
-             bup('ftp', input='ls src\n'))
+             bup('ftp', input='ls src\n').out)
 
     wvstart('cat')
     wvpasseq('excitement!\n',
-             bup('ftp', input='cat src/latest/file-1\n'))
+             bup('ftp', input='cat src/latest/file-1\n').out)
     wvpasseq('excitement!\nmore excitement!\n',
-             bup('ftp', input='cat src/latest/file-1 src/latest/dir/file-2\n'))
+             bup('ftp',
+                 input='cat src/latest/file-1 src/latest/dir/file-2\n').out)
     
     wvstart('get')
-    bupc('ftp', input=jl('get src/latest/file-1 dest'))
+    bup('ftp', input=jl('get src/latest/file-1 dest'))
     with open('dest', 'rb') as f:
         wvpasseq('excitement!\n', f.read())
     unlink('dest')
-    bupc('ftp', input=jl('get src/latest/file-symlink dest'))
+    bup('ftp', input=jl('get src/latest/file-symlink dest'))
     with open('dest', 'rb') as f:
         wvpasseq('excitement!\n', f.read())
     unlink('dest')
-    wvpassne(0, bupc('ftp',
-                     input=jl('get src/latest/bad-symlink dest'),
-                     check=False).rc)
-    wvpassne(0, bupc('ftp',
-                     input=jl('get src/latest/not-there'),
-                     check=False).rc)
+    wvpassne(0, bup('ftp',
+                    input=jl('get src/latest/bad-symlink dest'),
+                    check=False, stdout=None).rc)
+    wvpassne(0, bup('ftp',
+                    input=jl('get src/latest/not-there'),
+                    check=False, stdout=None).rc)
     
     wvstart('mget')
     unlink_if_exists('file-1')
-    bupc('ftp', input=jl('mget src/latest/file-1'))
+    bup('ftp', input=jl('mget src/latest/file-1'))
     with open('file-1', 'rb') as f:
         wvpasseq('excitement!\n', f.read())
     unlink_if_exists('file-1')
     unlink_if_exists('file-2')
-    bupc('ftp', input=jl('mget src/latest/file-1 src/latest/dir/file-2'))
+    bup('ftp', input=jl('mget src/latest/file-1 src/latest/dir/file-2'))
     with open('file-1', 'rb') as f:
         wvpasseq('excitement!\n', f.read())
     with open('file-2', 'rb') as f:
         wvpasseq('more excitement!\n', f.read())
     unlink_if_exists('file-symlink')
-    bupc('ftp', input=jl('mget src/latest/file-symlink'))
+    bup('ftp', input=jl('mget src/latest/file-symlink'))
     with open('file-symlink', 'rb') as f:
         wvpasseq('excitement!\n', f.read())
-    wvpassne(0, bupc('ftp',
-                     input=jl('mget src/latest/bad-symlink dest'),
-                     check=False).rc)
+    wvpassne(0, bup('ftp',
+                    input=jl('mget src/latest/bad-symlink dest'),
+                    check=False, stdout=None).rc)
     # bup mget currently always does pattern matching
-    bupc('ftp', input='mget src/latest/not-there\n')
+    bup('ftp', input='mget src/latest/not-there\n')

diff --git a/t/test-prune-older b/t/test-prune-older
index 95a26700434784755f638668c613c987e8eb1bdf..6adea5b80a6859e3eee57695a1f59d667bcec6c3 100755 (executable)
--- a/t/test-prune-older
+++ b/t/test-prune-older
@@ -23,23 +23,17 @@ sys.path[:0] = [abspath(script_home + '/../lib'), abspath(script_home + '/..')]
 top = os.getcwd()
 bup_cmd = top + '/bup'
 
-from buptest import exc, exo, test_tempdir
+from buptest import ex, exo, test_tempdir
 from wvtest import wvfail, wvpass, wvpasseq, wvpassne, wvstart
 
 from bup import compat
 from bup.helpers import partition, period_as_secs, readpipe
 
 
-def bup(*args):
-    return exo((bup_cmd,) + args).out
-
-def bupc(*args):
-    return exc((bup_cmd,) + args)
-
 def create_older_random_saves(n, start_utc, end_utc):
     with open('foo', 'w') as f:
         pass
-    exc(['git', 'add', 'foo'])
+    ex(['git', 'add', 'foo'])
     utcs = set()
     while len(utcs) != n:
         utcs.add(randint(start_utc, end_utc))
@@ -47,8 +41,8 @@ def create_older_random_saves(n, start_utc, end_utc):
     for utc in utcs:
         with open('foo', 'w') as f:
             f.write(str(utc) + '\n')
-        exc(['git', 'commit', '--date', str(utc), '-qam', str(utc)])
-    exc(['git', 'gc', '--aggressive'])
+        ex(['git', 'commit', '--date', str(utc), '-qam', str(utc)])
+    ex(['git', 'gc', '--aggressive'])
     return utcs
 
 # There is corresponding code in bup for some of this, but the
@@ -160,23 +154,23 @@ with test_tempdir('prune-older-') as tmpdir:
     now = int(time())
     three_years_ago = now - (60 * 60 * 24 * 366 * 3)
     chdir(tmpdir)
-    exc(['git', 'init', 'work'])
+    ex(['git', 'init', 'work'])
 
     wvstart('generating ' + str(save_population) + ' random saves')
     chdir(tmpdir + '/work')
     save_utcs = create_older_random_saves(save_population, three_years_ago, now)
     chdir(tmpdir)
     test_set_hash = exo(['git', 'show-ref', '-s', 'master']).out.rstrip()
-    ls_saves = bup('ls', 'master').splitlines()
+    ls_saves = exo((bup_cmd, 'ls', 'master')).out.splitlines()
     wvpasseq(save_population + 1, len(ls_saves))
 
     wvstart('ensure everything kept, if no keep arguments')
-    exc(['git', 'reset', '--hard', test_set_hash])
-    proc = exo((bup_cmd,
-                'prune-older', '-v', '--unsafe', '--no-gc',
-                '--wrt', str(now)) \
-               + ('master',),
-               stdout=None, stderr=PIPE, check=False)
+    ex(['git', 'reset', '--hard', test_set_hash])
+    proc = ex((bup_cmd,
+               'prune-older', '-v', '--unsafe', '--no-gc',
+               '--wrt', str(now)) \
+              + ('master',),
+              stdout=None, stderr=PIPE, check=False)
     wvpassne(proc.rc, 0)
     wvpass('at least one keep argument is required' in proc.err)
     check_prune_result(save_utcs)
@@ -189,19 +183,19 @@ with test_tempdir('prune-older-') as tmpdir:
                                     # some outside the save range.
                                     three_years_ago - period_scale['m'],
                                     now):
-        exc(['git', 'reset', '--hard', test_set_hash])
+        ex(['git', 'reset', '--hard', test_set_hash])
         expected = sorted(expected_retentions(save_utcs, now, spec))
-        exc((bup_cmd,
-             'prune-older', '-v', '--unsafe', '--no-gc', '--wrt', str(now)) \
-            + period_spec_to_period_args(spec) \
-            + ('master',))
+        ex((bup_cmd,
+            'prune-older', '-v', '--unsafe', '--no-gc', '--wrt', str(now)) \
+           + period_spec_to_period_args(spec) \
+           + ('master',))
         check_prune_result(expected)
 
 
     # More expensive because we have to recreate the repo each time
     wvstart('running %d generative gc tests on %d saves' % (prune_gc_cycles,
                                                             save_population))
-    exc(['git', 'reset', '--hard', test_set_hash])
+    ex(['git', 'reset', '--hard', test_set_hash])
     copytree('work/.git', 'clean-test-repo', symlinks=True)
     for spec in unique_period_specs(prune_gc_cycles,
                                     # Make it more likely we'll have
@@ -211,8 +205,8 @@ with test_tempdir('prune-older-') as tmpdir:
         rmtree('work/.git')
         copytree('clean-test-repo', 'work/.git')
         expected = sorted(expected_retentions(save_utcs, now, spec))
-        exc((bup_cmd,
-             'prune-older', '-v', '--unsafe', '--wrt', str(now)) \
-            + period_spec_to_period_args(spec) \
-            + ('master',))
+        ex((bup_cmd,
+            'prune-older', '-v', '--unsafe', '--wrt', str(now)) \
+           + period_spec_to_period_args(spec) \
+           + ('master',))
         check_prune_result(expected)