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: Prevent unittest --buffer from crashing #620

Merged
merged 1 commit into from Dec 30, 2023

Conversation

Jayman2000
Copy link
Contributor

@Jayman2000 Jayman2000 commented Dec 30, 2023

Before this change, if certain tests were failing in certain ways, then running python -m unittest --buffer would cause an AttributeError in the unittest module itself.

Here’s what unittest does when you use the --buffer argument:

  1. It sets sys.stdout and sys.stderr to StringIOs.
  2. It runs a test.
  3. If the test failed, it runs getvalue() on sys.stdout and sys.stderr to get data from its StringIOs.

tests/test_cli.py has its own RunContext class that does something similar. Before this change, here’s what could happen:

  1. unittest sets sys.stdout and sys.stderr to StringIOs.

  2. unittest runs a test that uses RunContext.

  3. A RunContext gets entered. It sets sys.stdout and sys.stderr to its own StringIOs.

  4. The RunContext gets exited. It sets sys.stdout and sys.stderr to sys.__stdout__ and sys.__stderr__.

  5. The test fails.

  6. unittest assumes that sys.stdout is still set to one of its StringIOs, and runs sys.stdout.getvalue().

  7. unittest crashes with this error:

    AttributeError: '_io.TextIOWrapper' object has no attribute 'getvalue'
    

@coveralls
Copy link

coveralls commented Dec 30, 2023

Coverage Status

coverage: 99.415% (+0.001%) from 99.414%
when pulling 37c2d5e on Jayman2000:unittest-buffer
into 52b40c8 on adrienverge:master.

Copy link
Owner

@adrienverge adrienverge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I confirm the problem (although Python documentation about sys.__stdout__ doesn't warn about potential issues).

And your fix seems to work!

I'm curious: why and when do you need --buffer?

tests/test_cli.py Show resolved Hide resolved
Before this change, if certain tests were failing in certain ways, then
running “python -m unittest --buffer” would cause an AttributeError in
the unittest module itself.

Here’s what unittest does when you use the --buffer argument:

1. It sets sys.stdout and sys.stderr to StringIOs [1].
2. It runs a test.
3. If the test failed, it runs getvalue() on sys.stdout and sys.stderr
   to get data from its StringIOs.

tests/test_cli.py has its own RunContext class that does something
similar. Before this change, here’s what could happen:

1. unittest sets sys.stdout and sys.stderr to StringIOs.
2. unittest runs a test that uses RunContext.
3. A RunContext gets entered. It sets sys.stdout and sys.stderr to its
   own StringIOs.
4. The RunContext gets exited. It sets sys.stdout and sys.stderr to
   sys.__stdout__ and sys.__stderr__.
5. The test fails.
6. unittest assumes that sys.stdout is still set to one of its
   StringIOs, and runs sys.stdout.getvalue().
7. unittest crashes with this error:

	AttributeError: '_io.TextIOWrapper' object has no attribute 'getvalue'

[1]: <https://github.com/python/cpython/blob/2305ca51448552542b2414186252123a8dc87db7/Lib/unittest/result.py#L65>
[2]: <https://github.com/python/cpython/blob/2305ca51448552542b2414186252123a8dc87db7/Lib/unittest/result.py#L87>
@Jayman2000
Copy link
Contributor Author

I'm curious: why and when do you need --buffer?

I don’t know. I tried running the tests on my local system when I ran into #621. I tried using --buffer as a part of my debugging process, and when it made things worse, I started investigating. I had only tried --buffer because it was mentioned on some StackOverflow question that I had found.

Copy link
Owner

@adrienverge adrienverge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK!

Thanks for contributing 👍

@adrienverge adrienverge merged commit 152ba20 into adrienverge:master Dec 30, 2023
7 checks passed
@Jayman2000 Jayman2000 deleted the unittest-buffer branch December 30, 2023 18:49
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

Successfully merging this pull request may close these issues.

None yet

3 participants