Tinche / aiofiles

File support for asyncio
Apache License 2.0
2.66k stars 150 forks source link

Move SpooledTemporaryFile.newlines from delegation to property #122

Closed cake-monotone closed 2 years ago

cake-monotone commented 2 years ago

Hi!

I found a minor issue in SpooledTemporaryFile and I fixed it.

Fix https://github.com/Tinche/aiofiles/issues/118

ref) https://github.com/python/cpython/blob/3e43fac2503afe219336742b150b3ef6e470686f/Lib/tempfile.py#L747

Tinche commented 2 years ago

Thanks! Does this have a test somewhere?

codecov-commenter commented 2 years ago

Codecov Report

Merging #122 (898cba4) into master (d010ff4) will increase coverage by 0.20%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #122      +/-   ##
==========================================
+ Coverage   89.59%   89.80%   +0.20%     
==========================================
  Files          10       10              
  Lines         298      304       +6     
==========================================
+ Hits          267      273       +6     
  Misses         31       31              
Impacted Files Coverage Δ
src/aiofiles/tempfile/temptypes.py 75.00% <100.00%> (ø)
src/aiofiles/os.py 100.00% <0.00%> (ø)
src/aiofiles/ospath.py 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d010ff4...898cba4. Read the comment docs.

cake-monotone commented 2 years ago

You're right :+1: I missed it.

Here's the missed test.

cake-monotone commented 2 years ago

hmm... it's weird

python3.6 's tempfile.SpooledTemporaryFile.newlines has diffrent implementation. (code)

cake-monotone commented 2 years ago

Because of other works, I think I'll be able to fix CI-fail tomorrow.

cake-monotone commented 2 years ago

Now, the test will be skipped if sys.version_info <= (3, 6)

Text-mode tempfile.SpooledTemporaryFile use StringIO in py3.6. (https://github.com/python/cpython/blob/v3.6.15/Lib/tempfile.py#L647) and StringIO doesn't implement newlines.

I think it's okay to skip py3.6

Tinche commented 2 years ago

Thanks!