summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTodd Gamblin <tgamblin@llnl.gov>2020-05-08 23:04:07 -0700
committerTodd Gamblin <tgamblin@llnl.gov>2020-05-09 00:56:18 -0700
commit7b8e5c89999621d3e036e52998a541c3a34e8fd0 (patch)
treee7a079e30870a95aa713b5c6e899f8407b8ad423
parent5883e9df53258f7fe32cbf76f14abe9e42dfeebc (diff)
downloadspack-7b8e5c89999621d3e036e52998a541c3a34e8fd0.tar.gz
spack-7b8e5c89999621d3e036e52998a541c3a34e8fd0.tar.bz2
spack-7b8e5c89999621d3e036e52998a541c3a34e8fd0.tar.xz
spack-7b8e5c89999621d3e036e52998a541c3a34e8fd0.zip
bugfix: don't use sys.stdout as a default arg value (#16541)
After migrating to `travis-ci.com`, we saw I/O issues in our tests -- tests that relied on `capfd` and `capsys` were failing. We've also seen this in GitHub actions, and it's kept us from switching to them so far. Turns out that the issue is that using streams like `sys.stdout` as default arguments doesn't play well with `pytest` and output redirection, as `pytest` changes the values of `sys.stdout` and `sys.stderr`. if these values are evaluated before output redirection (as they are when used as default arg values), output won't be captured properly later. - [x] replace all stream default arg values with `None`, and only assign stream values inside functions. - [x] fix tests we didn't notice were relying on this erroneous behavior
-rw-r--r--lib/spack/llnl/util/argparsewriter.py9
-rw-r--r--lib/spack/llnl/util/tty/color.py6
-rw-r--r--lib/spack/spack/cmd/commands.py3
-rw-r--r--lib/spack/spack/cmd/dependencies.py6
-rw-r--r--lib/spack/spack/cmd/dependents.py5
-rw-r--r--lib/spack/spack/cmd/find.py3
-rw-r--r--lib/spack/spack/cmd/modules/__init__.py3
-rw-r--r--lib/spack/spack/test/cmd/find.py2
8 files changed, 25 insertions, 12 deletions
diff --git a/lib/spack/llnl/util/argparsewriter.py b/lib/spack/llnl/util/argparsewriter.py
index f43595145e..8ecf6acc88 100644
--- a/lib/spack/llnl/util/argparsewriter.py
+++ b/lib/spack/llnl/util/argparsewriter.py
@@ -45,18 +45,18 @@ class Command(object):
class ArgparseWriter(argparse.HelpFormatter):
"""Analyzes an argparse ArgumentParser for easy generation of help."""
- def __init__(self, prog, out=sys.stdout, aliases=False):
+ def __init__(self, prog, out=None, aliases=False):
"""Initializes a new ArgparseWriter instance.
Parameters:
prog (str): the program name
- out (file object): the file to write to
+ out (file object): the file to write to (default sys.stdout)
aliases (bool): whether or not to include subparsers for aliases
"""
super(ArgparseWriter, self).__init__(prog)
self.level = 0
self.prog = prog
- self.out = out
+ self.out = sys.stdout if out is None else out
self.aliases = aliases
def parse(self, parser, prog):
@@ -167,7 +167,7 @@ _rst_levels = ['=', '-', '^', '~', ':', '`']
class ArgparseRstWriter(ArgparseWriter):
"""Write argparse output as rst sections."""
- def __init__(self, prog, out=sys.stdout, aliases=False,
+ def __init__(self, prog, out=None, aliases=False,
rst_levels=_rst_levels):
"""Create a new ArgparseRstWriter.
@@ -178,6 +178,7 @@ class ArgparseRstWriter(ArgparseWriter):
rst_levels (list of str): list of characters
for rst section headings
"""
+ out = sys.stdout if out is None else out
super(ArgparseRstWriter, self).__init__(prog, out, aliases)
self.rst_levels = rst_levels
diff --git a/lib/spack/llnl/util/tty/color.py b/lib/spack/llnl/util/tty/color.py
index 79f62c0040..b6f5ec782f 100644
--- a/lib/spack/llnl/util/tty/color.py
+++ b/lib/spack/llnl/util/tty/color.py
@@ -215,20 +215,22 @@ def cextra(string):
return len(''.join(re.findall(r'\033[^m]*m', string)))
-def cwrite(string, stream=sys.stdout, color=None):
+def cwrite(string, stream=None, color=None):
"""Replace all color expressions in string with ANSI control
codes and write the result to the stream. If color is
False, this will write plain text with no color. If True,
then it will always write colored output. If not supplied,
then it will be set based on stream.isatty().
"""
+ stream = sys.stdout if stream is None else stream
if color is None:
color = get_color_when()
stream.write(colorize(string, color=color))
-def cprint(string, stream=sys.stdout, color=None):
+def cprint(string, stream=None, color=None):
"""Same as cwrite, but writes a trailing newline to the stream."""
+ stream = sys.stdout if stream is None else stream
cwrite(string + "\n", stream, color)
diff --git a/lib/spack/spack/cmd/commands.py b/lib/spack/spack/cmd/commands.py
index 3664fce477..be934e0048 100644
--- a/lib/spack/spack/cmd/commands.py
+++ b/lib/spack/spack/cmd/commands.py
@@ -78,9 +78,10 @@ def setup_parser(subparser):
class SpackArgparseRstWriter(ArgparseRstWriter):
"""RST writer tailored for spack documentation."""
- def __init__(self, prog, out=sys.stdout, aliases=False,
+ def __init__(self, prog, out=None, aliases=False,
documented_commands=[],
rst_levels=['-', '-', '^', '~', ':', '`']):
+ out = sys.stdout if out is None else out
super(SpackArgparseRstWriter, self).__init__(
prog, out, aliases, rst_levels)
self.documented = documented_commands
diff --git a/lib/spack/spack/cmd/dependencies.py b/lib/spack/spack/cmd/dependencies.py
index 7f390341ef..bbccbe23ee 100644
--- a/lib/spack/spack/cmd/dependencies.py
+++ b/lib/spack/spack/cmd/dependencies.py
@@ -3,6 +3,8 @@
#
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+import sys
+
import llnl.util.tty as tty
from llnl.util.tty.colify import colify
@@ -43,7 +45,9 @@ def dependencies(parser, args):
spec = spack.cmd.disambiguate_spec(specs[0], env)
format_string = '{name}{@version}{%compiler}{/hash:7}'
- tty.msg("Dependencies of %s" % spec.format(format_string, color=True))
+ if sys.stdout.isatty():
+ tty.msg(
+ "Dependencies of %s" % spec.format(format_string, color=True))
deps = spack.store.db.installed_relatives(
spec, 'children', args.transitive, deptype=args.deptype)
if deps:
diff --git a/lib/spack/spack/cmd/dependents.py b/lib/spack/spack/cmd/dependents.py
index 89fd15ffda..5563701801 100644
--- a/lib/spack/spack/cmd/dependents.py
+++ b/lib/spack/spack/cmd/dependents.py
@@ -3,6 +3,8 @@
#
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+import sys
+
import llnl.util.tty as tty
from llnl.util.tty.colify import colify
@@ -84,7 +86,8 @@ def dependents(parser, args):
spec = spack.cmd.disambiguate_spec(specs[0], env)
format_string = '{name}{@version}{%compiler}{/hash:7}'
- tty.msg("Dependents of %s" % spec.cformat(format_string))
+ if sys.stdout.isatty():
+ tty.msg("Dependents of %s" % spec.cformat(format_string))
deps = spack.store.db.installed_relatives(
spec, 'parents', args.transitive)
if deps:
diff --git a/lib/spack/spack/cmd/find.py b/lib/spack/spack/cmd/find.py
index fa36e1cd26..db229aca7e 100644
--- a/lib/spack/spack/cmd/find.py
+++ b/lib/spack/spack/cmd/find.py
@@ -7,6 +7,7 @@ from __future__ import print_function
import copy
import os
+import sys
import llnl.util.tty as tty
import llnl.util.tty.color as color
@@ -236,7 +237,7 @@ def find(parser, args):
else:
if env:
display_env(env, args, decorator)
- if args.groups:
+ if sys.stdout.isatty() and args.groups:
tty.msg("%s" % plural(len(results), 'installed package'))
cmd.display_specs(
results, args, decorator=decorator, all_headers=True)
diff --git a/lib/spack/spack/cmd/modules/__init__.py b/lib/spack/spack/cmd/modules/__init__.py
index 6555b8c8a7..7d51224e14 100644
--- a/lib/spack/spack/cmd/modules/__init__.py
+++ b/lib/spack/spack/cmd/modules/__init__.py
@@ -119,8 +119,9 @@ _missing_modules_warning = (
" this command with debug output enabled for more details.")
-def loads(module_type, specs, args, out=sys.stdout):
+def loads(module_type, specs, args, out=None):
"""Prompt the list of modules associated with a list of specs"""
+ out = sys.stdout if out is None else out
# Get a comprehensive list of specs
if args.recurse_dependencies:
diff --git a/lib/spack/spack/test/cmd/find.py b/lib/spack/spack/test/cmd/find.py
index 8516569592..8d3c68d31a 100644
--- a/lib/spack/spack/test/cmd/find.py
+++ b/lib/spack/spack/test/cmd/find.py
@@ -324,7 +324,7 @@ def test_find_prefix_in_env(mutable_mock_env_path, install_mockery, mock_fetch,
def test_find_loaded(database, working_env):
output = find('--loaded', '--group')
- assert output == '' # 0 packages installed printed separately
+ assert output == ''
os.environ[uenv.spack_loaded_hashes_var] = ':'.join(
[x.dag_hash() for x in spack.store.db.query()])