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

lib/spack/llnl/util/argparsewriter.py

@@ -45,18 +45,18 @@ def __init__(self, prog, description, usage,
 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 @@ def write(self, parser):
 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 @@ def __init__(self, prog, out=sys.stdout, aliases=False,
             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
 

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)
 
 

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

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:

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,6 +86,7 @@ def dependents(parser, args):
         spec = spack.cmd.disambiguate_spec(specs[0], env)
 
         format_string = '{name}{@version}{%compiler}{/hash:7}'
+        if sys.stdout.isatty():
             tty.msg("Dependents of %s" % spec.cformat(format_string))
         deps = spack.store.db.installed_relatives(
             spec, 'parents', args.transitive)

lib/spack/spack/cmd/find.py

@@ -7,6 +7,7 @@
 
 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)

lib/spack/spack/cmd/modules/__init__.py

@@ -119,8 +119,9 @@ def one_spec_or_raise(specs):
     " 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:

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()])