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

fixture function makepyfile does not encode files properly #13331

Open
d-chris opened this issue Mar 27, 2025 · 2 comments
Open

fixture function makepyfile does not encode files properly #13331

d-chris opened this issue Mar 27, 2025 · 2 comments
Labels
topic: fixtures anything involving fixtures directly or indirectly type: enhancement new feature or API change, should be merged into features branch

Comments

@d-chris
Copy link

d-chris commented Mar 27, 2025

makepyfile is ignoring the encoding parameter, in both pytester and testdir fixtures.

found the issue working on windows with cp1252 encoding and using pytest-doctestplus.

i traced the issue to makepyfile, arguments and kwargs are passed unpacked.

 make_file = testdir.makepyfile(
            f"""
            def f():
                '''
                >>> print('{a}')
                {b}
                '''
                pass
            """.encode(encoding=encoding),
            encoding=encoding, # will be packed into `files` and default utf-8 encoding will be used
        )
$ python --version
Python 3.13.2

$ pip freeze
colorama==0.4.6
iniconfig==2.0.0
packaging==24.2
pluggy==1.5.0
pytest==8.3.3
@The-Compiler
Copy link
Member

The-Compiler commented Mar 27, 2025

I don't see a bug here, as such an argument simply doesn't exist, and neither is it valid to pass bytes in in the first place.

From the docs:

kwargs (str) – Each keyword is the name of a file, while the value of it will be written as contents of the file.

That passing in bytes works is a coincidence, and seems to be a historic artifact from Python 2 times (see e.g. f47ae74). The encoding= argument for _makefile was apparently added for pytest's selftests: d254c6b, but is not actually used since 4588653.

I think we should simply drop all the code dealing with bytes from Pytester._makefile and add proper type annotations to Pytester.makepyfile (not sure why everything but that has those!).

As for your tests, you should probably do something like this instead:

(pytester.path / "myfile.py").write_bytes(content)

@d-chris
Copy link
Author

d-chris commented Mar 27, 2025

for testing i agree, its no big issue.

basically _makefile was confusing me, it has a parameter encoding but kwargs are not unpacked instead passed on as dict seemed wrong to me.

def makepyfile(self, *args, **kwargs) -> Path:
    return self._makefile(".py", args, kwargs)

def _makefile(
  self,
  ext: str,                        # ".py"
  lines: Sequence[Any | bytes],    # contains args
  files: dict[str, str],           # contains kwargs
  encoding: str = "utf-8",         # useless? move into function as 'const'
) -> Path:
    ...

@Zac-HD Zac-HD added type: enhancement new feature or API change, should be merged into features branch topic: fixtures anything involving fixtures directly or indirectly labels Mar 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: fixtures anything involving fixtures directly or indirectly type: enhancement new feature or API change, should be merged into features branch
Projects
None yet
Development

No branches or pull requests

3 participants