Skip to content

bugfix: don't use sys.stdout as a default arg value#16541

Merged
tgamblin merged 2 commits intodevelopfrom
bugfix/capfd-capsys-in-travis
May 9, 2020
Merged

bugfix: don't use sys.stdout as a default arg value#16541
tgamblin merged 2 commits intodevelopfrom
bugfix/capfd-capsys-in-travis

Conversation

@tgamblin
Copy link
Copy Markdown
Member

@tgamblin tgamblin commented May 9, 2020

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.

  • replace all stream default arg values with None, and only assign stream values inside functions.
  • fix tests we didn't notice were relying on this erroneous behavior

@tgamblin tgamblin force-pushed the bugfix/capfd-capsys-in-travis branch from cc5483c to 1ed8b9f Compare May 9, 2020 06:15
@tgamblin tgamblin marked this pull request as ready for review May 9, 2020 06:20
@tgamblin tgamblin changed the title bugfix: work around travis I/O issues bugfix: don't use streams as default arg values May 9, 2020
@tgamblin tgamblin changed the title bugfix: don't use streams as default arg values bugfix: don't use sys.stdout as a default arg value May 9, 2020
@tgamblin tgamblin force-pushed the bugfix/capfd-capsys-in-travis branch from 1ed8b9f to df5585b Compare May 9, 2020 07:24
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
@tgamblin tgamblin force-pushed the bugfix/capfd-capsys-in-travis branch from df5585b to 11afc06 Compare May 9, 2020 07:24
@tgamblin tgamblin requested a review from alalazo May 9, 2020 07:27
@tgamblin tgamblin merged commit 7b8e5c8 into develop May 9, 2020
@alalazo alalazo deleted the bugfix/capfd-capsys-in-travis branch May 9, 2020 08:52
@alalazo
Copy link
Copy Markdown
Member

alalazo commented May 9, 2020

🥳

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.

3 participants