filesystem: Wrap archived files inside TextIOWrapper

aconrad / pycobertura

A code coverage diff tool for Cobertura reports

MIT License

116 stars 39 forks source link

filesystem: Wrap archived files inside TextIOWrapper #169

Closed ernestask closed 1 year ago

ernestask commented 1 year ago

Currently, when mixing DirectoryFileSystem and ZipFileSystem, diffing reports wrong results for identical coverage reports due to differing types in file systems (byte-string lines compared with text lines).

This commit wraps the files opened from a zip archive inside TextIOWrapper to avoid the issue.

Closes https://github.com/aconrad/pycobertura/issues/107

aconrad commented 1 year ago

Thanks for tackling this issue @ernestask, much appreciated. 🙏

Is there a way we can demonstrate this fix in tests/test_filesystem.py?
How about tests/test_cobertura_diff.py? I see you already posted a ZIP file that includes a reproducible scenario for this issue, thank you! Could we use that in the tests/ directory to demonstrate the issue for a diff test?

ernestask commented 1 year ago

Hey, yeah, I’ll look into that tomorrow.

aconrad commented 1 year ago

Additionally, can we add an entry under ## Unreleased in CHANGES.md that explains what was fixed?

ernestask commented 1 year ago

Added some tests. Didn’t need to add another source archive, the dummy one can be used as well.

ernestask commented 1 year ago

:pray:

Do you think .1 could be tagged with this? I’d start using it right away in that case.

aconrad commented 1 year ago

https://pypi.org/project/pycobertura/3.2.1/

ernestask commented 1 year ago

Awesome, thanks a lot!