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

Avoid ResourceWarnings during tests for /dev/null handle #879

Merged
merged 1 commit into from
Sep 28, 2022

Conversation

rmartin16
Copy link
Member

@rmartin16 rmartin16 commented Sep 28, 2022

After #867, pytest will raise for all Python errors including those normally suppressed. This change is also causing the errors/warnings below to be printed sometimes when pytest is run.

(venv3.10) rkm@eunectes briefcase % pytest tests/commands/new/test_call.py 
..
------------------------------------------------------------------------------
Ran 2 tests in 0.01s

OK
Exception ignored in: <_io.FileIO name='/dev/null' mode='wb' closefd=True>
ResourceWarning: unclosed file <_io.TextIOWrapper name='/dev/null' mode='w' encoding='utf-8'>

These changes ensure the file handle for /dev/null/ that's opened for the logfile Rich Console is closed before pytest` exits so this error is showed to developers and avoids subsequent confusion.

In practice, these errors are innocuous. Furthermore, end-users cannot see these errors unless they run a debug version of Python or use the -Werror (or perhaps the -Wdefault) flag.

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

# Rich does this to prevent individual words being split between
# two lines in the terminal...however, these additional line breaks
# cause some tests to fail unexpectedly.
Printer.console.soft_wrap = True
Copy link
Member Author

Choose a reason for hiding this comment

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

Setting soft_wrap=True is no longer necessary since we always set it to True in console.py.

@rmartin16 rmartin16 marked this pull request as ready for review September 28, 2022 02:07
@codecov
Copy link

codecov bot commented Sep 28, 2022

Codecov Report

Merging #879 (3c428fa) into main (9ab31c8) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted Files Coverage Δ
src/briefcase/console.py 100.00% <100.00%> (ø)
src/briefcase/commands/package.py 86.04% <0.00%> (ø)
src/briefcase/integrations/subprocess.py 100.00% <0.00%> (ø)
src/briefcase/commands/new.py 97.01% <0.00%> (+0.02%) ⬆️
src/briefcase/commands/publish.py 96.66% <0.00%> (+0.51%) ⬆️

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

👍 Seems reasonable to me!

@freakboy3742 freakboy3742 merged commit e7b0ef7 into beeware:main Sep 28, 2022
@rmartin16 rmartin16 deleted the close-dev-null branch January 21, 2023 16:50
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

2 participants