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

Fix test get subprocess output interleaved py3k #6713

Merged
merged 3 commits into from Nov 1, 2018

Conversation

Projects
None yet
2 participants
@OniOni
Contributor

OniOni commented Nov 1, 2018

Problem

While running our unit tests in python 3, test_get_subprocess_output_interleaved started consistently failing with:

==================== FAILURES ====================
 TestProcessManager.test_get_subprocess_output_interleaved 

self = <pants_test.pantsd.test_process_manager.TestProcessManager testMethod=test_get_subprocess_output_interleaved>

    def test_get_subprocess_output_interleaved(self):
      cmd_payload = 'import sys; ' + ('sys.stderr.write("9"); sys.stdout.write("3"); ' * 3)
      cmd = [sys.executable, '-c', cmd_payload]
    
      self.assertEqual(self.pm.get_subprocess_output(cmd), '333')
>     self.assertEqual(self.pm.get_subprocess_output(cmd, ignore_stderr=False), '939393')
E     AssertionError: '333999' != '939393'
E     - 333999
E     + 939393

pants_test/pantsd/test_process_manager.py:222: AssertionError
-------------- Captured stderr call --------------
999

Also see #6385 for more context

Solution

Fix test.

Inline message for from fixing commit (48c478e):

This one need some explaining.

This arised because Python 3 makes `sys.std{err,out}` a
`TextIOWrapper` and as per the python 3 man page "The text I/O layer
will still be line-buffered.". And that's with `-u` or
`PYTHONUNBUFFERED` set.

BUT, this test is not checking that python is buffering or not. It's
checking that if we run an executable in a subrocess for which we
capture output, we're correctly reading the potentially interleaved
STDIN and STDERR.

Unfortunatly, the "executable" we're using to test the capturing was
broken by the change to python 3. This change fixes the executable.

I also added a test to try and make potential future failures easier to understand.

Result

  • Failure is resolved.

OniOni added some commits Oct 31, 2018

[fix] Fix `test_get_subprocess_output_interleaved` in python 3.
This one need some explaining.

This arised because Python 3 makes `sys.std{err,out}` a
`TextIOWrapper` and as per the python 3 man page "The text I/O layer
will still be line-buffered.". And that's with `-u` or
`PYTHONUNBUFFERED` set.

BUT, this test is not checking that python is buffering or not. It's
checking that if we run an executable in a subrocess for which we
capture output, we're correctly reading the potentially interleaved
STDIN and STDERR.

Unfortunatly, the "executable" we're using to test the capturing was
broken by the change to python 3. This change fixes the executable.
[feat] Add second interleave test using bash executable.
This let's us seperate failure in executable vs. failure in capture
behaviours.
@stuhood

stuhood approved these changes Nov 1, 2018

Thanks a bunch!

@stuhood stuhood merged commit c2f2ef6 into pantsbuild:master Nov 1, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment