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

Add type-hinting, newer Python versions, and test on Windows. #62

Merged
merged 6 commits into from
May 15, 2023

Conversation

HexDecimal
Copy link
Collaborator

Many tests were failing when run on Windows. I have fixed them and marked the rest:

  • Windows paths were being added to test code without being escaped correctly.
  • Windows does not treat Python scripts as executables for shutil.which, so tests relying on this will fail.
  • The Windows SYSTEMROOT environment variable is required to initialize Python correctly. "subprocess" mode often crashes without it. This has been fixed.
  • easy_install has encoding issues on Windows. It's also missing from many environments so I've replaced the test using it with a custom script.

Updated some older pytest fixtures to newer ones. Most of the current fixtures have replacements and I skipped over a lot of them.

Added Mypy to Tox. I've fully typed the code so that it passed mypy --strict but I didn't add --strict to the linting tests yet.

Added Python versions up to 3.11 to CI, also added Windows to CI.

I tested coverage, only two lines in pytest_console_scripts.py are missing from a Windows test.

Fix and mark issues with Windows.
Windows paths were being added to code without being escaped correctly.
Windows does not treat Python scripts as executables for `shutil.which`,
so tests relying on this will fail.

Update older pytest fixtures to newer ones.
Add Mypy to Tox.

Currently no issues are found with `mypy --strict`.
`easy_install` is missing from too many environments to be used in
tests and has encoding issues on Windows.
It has been replaced with a custom script containing `sys.exit(None)`.
Coverage showed that no tests were using `sys.exit(<int>)`.
@HexDecimal
Copy link
Collaborator Author

I fixed the Windows env issues by copying the SYSTEMROOT environment variable in the run_subprocess method, but now that I think about it, it might've better to fix this in the tests instead by working on a copy of the env instead of a blank one.

The issue might be too specific to require this fix.
Something like a warning might be better.
Copy link
Owner

@kvas-it kvas-it left a comment

Choose a reason for hiding this comment

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

Thanks, this is great!

I have a question about introducing RunTest class in the test (see inline) but I don't feel particularly strongly about it. Let me know what you think.

Cheers,
Vasily

tests/test_console_scripts.py Show resolved Hide resolved
@kvas-it kvas-it merged commit 7c0be8a into kvas-it:master May 15, 2023
@HexDecimal HexDecimal deleted the type-hinting branch May 16, 2023 00:59
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.

2 participants