remove uv errors when creating a new project on windows. #3
Conversation
…rue is not a bad fix
| # the packages are installed into the correct virtual environment. | ||
| # In testing, when this was not set, packages could end up in the pyenv's | ||
| # site-packages directory; it's possible an environment variable can control this. | ||
| def _get_executeable_paths(): |
There was a problem hiding this comment.
I had just done this when the _python_executable PR merged in. I'm not sure if my version is worse than the other one, but since both paths mattered based on the OS I smashed them into one func at the top of the file.
| try: | ||
| while process.poll() is None: | ||
| try: | ||
| stdout, stderr = process.communicate(timeout=1.0) |
There was a problem hiding this comment.
process.communicate is the solution to the main issue on this PR.
Using it required changing around the std in/out handling a touch, but the internet tells me .communicate is more cross-platform compatible. I'll be testing on my mac when I find the password -.-
| process.terminate() | ||
| except: | ||
| pass | ||
| except Exception as e: |
There was a problem hiding this comment.
ruff barked at me for an naked except so I tossed this in here. If my linter is the problem I'd love to know how to make ruff obey the pyproject.toml. It also happens if I run the pre-commit like I usually do.
There was a problem hiding this comment.
Yeah the right move is to have it catch something that is of type Exception which is everything. If you any to go into best practice mode, it would be catch specific exceptions, but a catch all is fine if it doesn't get hidden and out the app in a weird state.
| def install(package: str): | ||
| """Install a package with `uv` and add it to pyproject.toml.""" | ||
| global _python_executable | ||
| global _PYTHON_EXECUTABLE |
There was a problem hiding this comment.
Any chance this constant will change? The only reason to use global is if the function will mutate it causing it to function scope.
| process.terminate() | ||
| except: | ||
| pass | ||
| except Exception as e: |
There was a problem hiding this comment.
Yeah the right move is to have it catch something that is of type Exception which is everything. If you any to go into best practice mode, it would be catch specific exceptions, but a catch all is fine if it doesn't get hidden and out the app in a weird state.
| BASE_PATH = Path(__file__).parent | ||
|
|
||
|
|
||
| class CLIInitTest(unittest.TestCase): |
There was a problem hiding this comment.
I would use functional pytest patterns and fixtures within conftests etc instead of using unittest.TestCase. It's the more supported pattern now a days and much easier to expand with reusable fixtures etc as the test suite grows.
📥 Pull Request
Windows errors on cli completion :(
📘 Description
🧪 Testing
All the tests with changes here fail by default on windows when running whatever flavor of the
toxcommand you feel like. While going thru each one, it seems largely to be how the filesystem deals with existing files. I'm still trying to sus out any bad ideas in the tests since my pytest game is mid and my Cypress game is strong.