Skip to content

Conversation

@d-chris
Copy link
Contributor

@d-chris d-chris commented Mar 19, 2025

see: fixture testdir.makepyfile does not encode files properly #285

hopefully the tests will show there is something odd, and someone can fix it

run pytests:

pytest tests\test_makepyfile.py::test_compare_make_basic_file

see: fixture `testdir.makepyfile` does not encode files properly scientific-python#285
d-chris and others added 3 commits March 19, 2025 22:58
Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com>
Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com>
Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com>

text = Path(file).read_text(enc)

print(text, diff)
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

set env variable PYTEST_DEBUG_MAKEPYFILE to run test function

@pllim pllim requested a review from bsipocz March 20, 2025 16:55
@pllim
Copy link
Contributor

pllim commented Mar 20, 2025

Thanks for adding the tests! Some comments posted and hopefully more eyes can look at this.

@d-chris
Copy link
Contributor Author

d-chris commented Mar 23, 2025

in my generated test content i discovered a bug which i fixed in ef57b66

also debug tests are skipped in CI enable with env PYTEST_DEBUG_MAKEPYFILE c8f55a2

@d-chris
Copy link
Contributor Author

d-chris commented Mar 24, 2025

@bsipocz you don' t have to run the CI, test are still failing

somehow makepyfile uses not the given encoding utf-16.

I don't know if further debugging makes any sense. I guess testdir is a legacy feature and should no longer be used.

FAILED tests/test_makepyfile.py::test_compare_write_bytes[utf_16-letters] - AssertionError: makepyfile screwed up encoding='utf_16' and raised error=UnicodeDecodeError('utf-8', b"\xff\xfe\n\x00d\x00e\x00f\x00 \x00f\x00(\x00)\x00:\x00\n\x00 \x00 \x00 \x00 \x00'\x00'\x00'\x00\n\x00 \x00 \x00 \x00 \x00>\x00>\x00>\x00 \x00p\x00r\x00i\x00n\x00t\x00(\x00'\x00A\x00'\x00)\x00\n\x00 \x00 \x00 \x00 \x00B\x00\n\x00 \x00 \x00 \x00 \x00'\x00'\x00'\x00\n\x00 ...

@bsipocz
Copy link
Member

bsipocz commented Mar 24, 2025

Ahh, OK. I suppose I was pushing the buttons faster than I was reading the comments ;)

@d-chris
Copy link
Contributor Author

d-chris commented Mar 26, 2025

problem is located in pytest, guess have to open an issue there.

nothing we can do, except dont use fixture testdir.makepyfile when encoding is required

pytets

required fix is unpack args and kwargs: return self._makefile(".py", *args, **kwargs)

usage

files is a positional argument and then has to be passed on

make_file = testdir.makepyfile(
    bytes_content,
    files={},
    encoding=encoding,
)

@bsipocz
Copy link
Member

bsipocz commented Mar 27, 2025

Upstream basically said not to do this, so I'm closing the PR as Christoph suggested.

Thanks for all the work you put in the investigation!

@bsipocz bsipocz closed this Mar 27, 2025
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