Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests shouldn't be printing to stderr #6831

Closed
cosmicexplorer opened this issue Nov 29, 2018 · 1 comment
Closed

tests shouldn't be printing to stderr #6831

cosmicexplorer opened this issue Nov 29, 2018 · 1 comment

Comments

@cosmicexplorer
Copy link
Contributor

cosmicexplorer commented Nov 29, 2018

In the investigation of #6828, it was difficult to parse log output for legitimate error messages because of the variety of messages printed directly to stderr from passing tests. For example, pants_test.backend.python.tasks.test_python_distribution_integration.PythonDistributionIntegrationTest#test_binary_dep_isolation_with_multiple_targets prints unconditionally:

Traceback (most recent call last):
  File ".bootstrap/_pex/pex.py", line 349, in execute
  File ".bootstrap/_pex/pex.py", line 281, in _wrap_coverage
  File ".bootstrap/_pex/pex.py", line 312, in _wrap_profiling
  File ".bootstrap/_pex/pex.py", line 394, in _execute
  File ".bootstrap/_pex/pex.py", line 506, in execute_entry
  File ".bootstrap/_pex/pex.py", line 513, in execute_module
  File "/opt/twitter_mde/package/eepython27/e4cdee382b83e4185a42b54120b1b11021cec1d8285cc9023ef2dabf8c9af24d/lib/python2.7/runpy.py", line 192, in run_module
    fname, loader, pkg_name)
  File "/opt/twitter_mde/package/eepython27/e4cdee382b83e4185a42b54120b1b11021cec1d8285cc9023ef2dabf8c9af24d/lib/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "python_distribution/fasthello_with_install_requires/main.py", line 8, in <module>
ImportError: No module named pycountry

The relevant test output from #6828 is in this gist, but I suspect there are more such tests. We should figure out how to stop or redirect sources of printing directly to stderr in these tests to make it easier to read logs.

One relevant point which is also visible in that gist is that pex will unconditionally print the output of building a python_dist() which has native Extensions in its setup.py directly to stderr, and this has generated encoding errors internally in the past (where if the compilation failed, you couldn't see the error, because pex would print directly to stderr, but the act of printing to stderr raised an exception). That part would require a small and I believe uncontroversial change somewhere around this line of pex/installer.py.

@Eric-Arellano
Copy link
Contributor

Closing as it was decided in #9710 (comment) that we should indeed be writing to stderr, per John's coined "emoji rule".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants