klein icon indicating copy to clipboard operation
klein copied to clipboard

Use file context manager to close files in a timely manner

Open exarkun opened this issue 3 years ago • 4 comments

The test suite reports ResourceWarning from two places in the test suite. This fixes those cases.

exarkun avatar Aug 27 '22 17:08 exarkun

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (57d0427) 99.04% compared to head (34149e0) 99.04%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #564   +/-   ##
=======================================
  Coverage   99.04%   99.04%           
=======================================
  Files          44       44           
  Lines        3881     3883    +2     
  Branches      517      519    +2     
=======================================
+ Hits         3844     3846    +2     
  Misses         23       23           
  Partials       14       14           
Impacted Files Coverage Δ
src/klein/test/test_resource.py 98.35% <100.00%> (+<0.01%) :arrow_up:

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov[bot] avatar Aug 27 '22 21:08 codecov[bot]

I don't know why this is failing on CI.

And I can't reproduce it on my local dev env... but even if I put a broken config link, it still fails on my local dev.

$ sphinx-build -b html -aEvW docs/ htmldocs
Running Sphinx v5.1.1
building [mo]: all of 0 po files
building [html]: all source files
updating environment: [new config] 16 added, 0 changed, 0 removed
reading sources... [100%] release                                                                         
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
writing output... [100%] release                                                                          
generating indices... genindex done
writing additional pages... search done
copying downloadable files... [100%] introduction/codeexamples/template.py                                
copying static files... done
copying extra files... done
dumping search index in English (code: en)... done
dumping object inventory... done
build succeeded.

The HTML pages are in htmldocs.
(venv) ✔  ~/chevah/klein [quiet-resource-warnings|✔ ] 

10:46 $ git top
63f7178 Adi Roiban 2022-08-28 Use RTD link for API extension.
7f536e5 Jean-Paul Calderone 2022-08-27 Fix obvious refactoring mistake

adiroiban avatar Aug 28 '22 09:08 adiroiban

I have pushed some changes to this PR...but think is better to remove them.

I will try to help fixing https://github.com/twisted/klein/pull/565

@exarkun let me know if you want me to rewrite the history and remove the last 2 commits from this PR

adiroiban avatar Aug 28 '22 10:08 adiroiban

I have pushed some changes to this PR...but think is better to remove them.

I will try to help fixing #565

@exarkun let me know if you want me to rewrite the history and remove the last 2 commits from this PR

Thanks for looking into this @adiroiban . Since the changes didn't fix the issue, removing them makes sense. It doesn't make any difference to me whether they're removed with history rewriting or not. I usually avoid history rewriting for public branches but force pushing HEAD^^ sounds quite low-effort so if that's your preference please go ahead - it won't cause any issues for me.

exarkun avatar Aug 28 '22 12:08 exarkun

It's approved and the issues on master that were gumming up the works are now addressed, so hopefully this will cleanly auto-merge after CI has done its thing.

glyph avatar Apr 28 '23 20:04 glyph