[PATCH 1/1] Unify some of the test subprocess invocation functions

[Rob Browning]

We may eventually want to simplify further, but for now, decrease the
number of separate commands used to run test subprocesses. Drop
buptest.run and migrate a number of other ex/exo/exc implementations
to buptest exc and exo (both handled in the end via subprocess.run).

Since it's easy enough, rename buptest ex to exc to avoid our common
"except ... as ex" binding from shadowing it (cf. pylint
redefined-outer-name).

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

Pushed to main.

test/ext/test_ftp.py | 2 +-
test/ext/test_get.py | 5 +--
test/ext/test_prune_older.py | 2 +-
test/ext/test_split_trees.py | 2 +-
test/int/test_client.py | 2 +-
test/int/test_commit.py | 14 ++-----
test/int/test_git.py | 13 ++-----
test/int/test_metadata.py | 2 +-
test/int/test_resolve.py | 2 +-
test/int/test_treesplit.py | 2 +-
test/int/test_vfs.py | 8 +---
test/lib/buptest/__init__.py | 74 ++++++++++++++++--------------------
12 files changed, 50 insertions(+), 78 deletions(-)

diff --git a/test/ext/test_ftp.py b/test/ext/test_ftp.py
index 3fb6467a..49892a8a 100644
--- a/test/ext/test_ftp.py
+++ b/test/ext/test_ftp.py
@@ -6,7 +6,7 @@ import re

from bup.compat import environ
from bup.helpers import unlink as unlink_if_exists
-from buptest import ex, exo
+from buptest import exc as ex, exo
from wvpytest import wvfail, wvpass, wvpasseq, wvpassne, wvstart
import bup.path

diff --git a/test/ext/test_get.py b/test/ext/test_get.py
index 99c30b2b..56e9d2ee 100644
--- a/test/ext/test_get.py
+++ b/test/ext/test_get.py
@@ -10,7 +10,7 @@ from bup import compat, path
from bup.compat import environ
from bup.helpers import EXIT_FAILURE, bquote, unlink
from bup.io import byte_stream
-from buptest import ex, exo
+from buptest import exc as ex, exo
from wvpytest import wvcheck, wvfail, wvmsg, wvpass, wvpasseq, wvpassne, wvstart
import bup.path

@@ -57,8 +57,7 @@ def verify_rcz(cmd, **kwargs):
kwargs['check'] = False
result = exo(cmd, **kwargs)
stdout.write(result.out)
- rc = result.proc.returncode
- wvcheck(rc == 0, 'process exit %d == 0' % rc)
+ wvcheck(result.rc == 0, f'process exit {result.rc:d} == 0')
return result

# FIXME: multline, or allow opts generally?
diff --git a/test/ext/test_prune_older.py b/test/ext/test_prune_older.py
index 54c0d585..92e1614e 100644
--- a/test/ext/test_prune_older.py
+++ b/test/ext/test_prune_older.py
@@ -14,7 +14,7 @@ from bup import compat
from bup.compat import environ
from bup.helpers import partition, period_as_secs
from bup.io import byte_stream
-from buptest import ex, exo
+from buptest import exc as ex, exo
from wvpytest import wvfail, wvpass, wvpasseq, wvpassne, wvstart
import bup.path

diff --git a/test/ext/test_split_trees.py b/test/ext/test_split_trees.py
index 56508204..abdb9019 100755
--- a/test/ext/test_split_trees.py
+++ b/test/ext/test_split_trees.py
@@ -3,7 +3,7 @@ from os import chdir, environb as environ
from os.path import join as joinp
import pytest

-from buptest import ex, exo
+from buptest import exc as ex, exo
import bup.path


diff --git a/test/int/test_client.py b/test/int/test_client.py
index 38499125..96379a32 100644
--- a/test/int/test_client.py
+++ b/test/int/test_client.py
@@ -2,7 +2,7 @@
import os, time, random, subprocess, glob
import pytest

-from buptest import ex
+from buptest import exc as ex

from bup import client, git, path
from bup.compat import environ
diff --git a/test/int/test_commit.py b/test/int/test_commit.py
index c9396567..df130cd4 100644
--- a/test/int/test_commit.py
+++ b/test/int/test_commit.py
@@ -1,22 +1,16 @@

from os import environb as environ
-from subprocess import check_call, check_output
-import os, sys
+import os

from wvpytest import *
+import buptest

from bup import git
from bup.commit import _git_date_str, has_trailers, parse_commit
-from bup.io import path_msg


-def exc(*cmd):
- print(' '.join(path_msg(x) for x in cmd), file=sys.stderr)
- check_call(cmd)
-
-def exo(*cmd):
- print(' '.join(path_msg(x) for x in cmd), file=sys.stderr)
- return check_output(cmd)
+def exc(*cmd): return buptest.exc(cmd)
+def exo(*cmd): return buptest.exo(cmd).out


def test_commit_parsing(tmpdir):
diff --git a/test/int/test_git.py b/test/int/test_git.py
index f89ec874..29307657 100644
--- a/test/int/test_git.py
+++ b/test/int/test_git.py
@@ -1,31 +1,24 @@

-import sys
from binascii import hexlify, unhexlify
from contextlib import ExitStack
from functools import partial
-from subprocess import check_call, check_output
from time import localtime
import struct, os
import pytest

from wvpytest import *
+import buptest

from bup import git, path
from bup.compat import environ
from bup.helpers import OBJECT_EXISTS, finalized, log, mkdirp
-from bup.io import path_msg


bup_exe = path.exe()


-def exc(*cmd):
- print(' '.join(path_msg(x) for x in cmd), file=sys.stderr)
- check_call(cmd)
-
-def exo(*cmd):
- print(' '.join(path_msg(x) for x in cmd), file=sys.stderr)
- return check_output(cmd)
+def exc(*cmd): return buptest.exc(cmd)
+def exo(*cmd): return buptest.exo(cmd).out


def local_writer():
diff --git a/test/int/test_metadata.py b/test/int/test_metadata.py
index c3b77213..54510290 100644
--- a/test/int/test_metadata.py
+++ b/test/int/test_metadata.py
@@ -22,7 +22,7 @@ bup_path = top_dir + b'/bup'


def ex(*args, **kwargs):
- return buptest.ex(args, **kwargs)
+ return buptest.exc(args, **kwargs)


def setup_testfs():
diff --git a/test/int/test_resolve.py b/test/int/test_resolve.py
index abbd4075..8594f5fb 100644
--- a/test/int/test_resolve.py
+++ b/test/int/test_resolve.py
@@ -6,7 +6,7 @@ from stat import S_IFDIR
import os
from time import localtime, strftime

-from buptest import ex, exo
+from buptest import exc as ex, exo
from buptest.vfs import tree_dict
from wvpytest import *

diff --git a/test/int/test_treesplit.py b/test/int/test_treesplit.py
index 5a90f045..84b496be 100644
--- a/test/int/test_treesplit.py
+++ b/test/int/test_treesplit.py
@@ -4,7 +4,7 @@ from pathlib import Path
from sys import stderr
import re

-from buptest import ex, exo
+from buptest import exc as ex, exo
from wvpytest import *

from bup import git, tree
diff --git a/test/int/test_vfs.py b/test/int/test_vfs.py
index df6cb337..e506adb6 100644
--- a/test/int/test_vfs.py
+++ b/test/int/test_vfs.py
@@ -3,20 +3,18 @@ from binascii import unhexlify
from os import symlink
from random import Random, randint
from stat import S_IFDIR, S_IFLNK, S_IFREG, S_ISDIR, S_ISREG
-from sys import stderr
from time import localtime, strftime, tzset
import os, sys

import pytest

-from buptest import exo
+from buptest import exc as ex, exo
from buptest.vfs import tree_dict
from wvpytest import *

from bup._helpers import write_random
from bup import git, metadata, vfs
from bup.compat import environ, fsencode
-from bup.helpers import exc, shstr
from bup.metadata import Metadata
from bup.repo import LocalRepo

@@ -24,10 +22,6 @@ lib_t_dir = os.path.dirname(fsencode(__file__))
top_dir = os.path.join(lib_t_dir, b'../..')
bup_path = top_dir + b'/bup'

-def ex(cmd, **kwargs):
- print(shstr(cmd), file=stderr)
- return exc(cmd, **kwargs)
-
def test_default_modes():
wvpasseq(S_IFREG | 0o644, vfs.default_file_mode)
wvpasseq(S_IFDIR | 0o755, vfs.default_dir_mode)
diff --git a/test/lib/buptest/__init__.py b/test/lib/buptest/__init__.py
index f8322ec2..4bce7960 100644
--- a/test/lib/buptest/__init__.py
+++ b/test/lib/buptest/__init__.py
@@ -2,61 +2,53 @@
from collections import namedtuple
from os.path import abspath, basename, dirname, realpath
from shlex import quote
-from subprocess import PIPE, Popen
+from subprocess import PIPE, check_output
from traceback import extract_stack
import errno, os, subprocess, sys, tempfile

from bup import helpers
from bup.compat import fsencode
-from bup.io import byte_stream
+from bup.io import enc_shs


-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'
- % (cmd, p.returncode,
- (', stderr: %r' % err) if err else ''))
- return ex_res(out=out, err=err, proc=p, rc=p.returncode)
-
def logcmd(cmd):
- s = helpers.shstr(cmd)
+ def cvt(x):
+ if isinstance(x, bytes):
+ return enc_shs(x).decode('ascii')
+ if isinstance(x, str):
+ return enc_shs(x)
+ assert False, x
if isinstance(cmd, str):
- print(s, file=sys.stderr)
- else:
- # bytes - for now just escape it
- print(s.decode(errors='backslashreplace'), file=sys.stderr)
+ print(' '.join(cvt(x) for x in cmd), file=sys.stderr)
+
+ex_res = namedtuple('SubprocResult', ('out', 'err', 'rc'))
+
+def exc(cmd, **kwargs):
+ """Print cmd to stderr and then run it via subprocess.run(), but
+ with a default of check=True. If the command's stderr is
+ captured, and there's any data, write it to stderr. Return a
+ corresponding ex_res().

-def ex(cmd, **kwargs):
- """Print cmd to stderr and then run it as per run(...).
- Print the subprocess stderr to stderr if stderr=PIPE and there's
- any data.
"""
logcmd(cmd)
- result = run(cmd, **kwargs)
- if result.err:
+ kwargs.setdefault('check', True)
+ cp = subprocess.run(cmd, **kwargs)
+ if cp.stderr:
sys.stderr.flush()
- byte_stream(sys.stderr).write(result.err)
- return result
+ if isinstance(cp.stderr, bytes):
+ sys.stderr.buffer.write(cp.stderr)
+ else:
+ sys.stderr.write(cp.stderr)
+ return ex_res(out=cp.stdout, err=cp.stderr, rc=cp.returncode)

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.
+ """Print cmd to stderr and then run it via ex(cmd, stdout=PIPE,
+ ...). Return a corresponding ex_res().

"""
- assert 'stdout' not in kwargs
- kwargs['stdout'] = PIPE
- return ex(cmd, **kwargs)
+ logcmd(cmd)
+ if 'stdout' not in kwargs:
+ kwargs['stdout'] = PIPE
+ else:
+ assert kwargs['stdout'] == PIPE, kwargs
+ return exc(cmd, **kwargs)
--
2.47.3