specify pythonpath for pytest #123
Conversation
| @@ -28,3 +28,4 @@ where = src | |||
|
|
|||
| [tool:pytest] | |||
| python_files = *_test.py test_*.py *_example.py example_*.py | |||
| pythonpath = src | |||
README.md
Outdated
| @@ -16,7 +16,7 @@ This project includes pytest unit test and llvm-style filecheck tests. They can | |||
|
|
|||
| ``` | |||
| # Executes pytests which are located in tests/ | |||
| pytest | |||
| python -m pytest | |||
There was a problem hiding this comment.
I am surprised this causes a difference. Are you sure this change is needed, and the change below is not sufficient by itself. If both are needed, we should probably change the documentation as suggested here.
There was a problem hiding this comment.
This isn't necessary but my impression is that it's the recommended best practice. Python versions and tools can go out of sync for reasons that are still unclear to me, and my local pytest was not the same as python -m pytest. This change guarantees that they'll be the same, even if it's a bit more verbose
There was a problem hiding this comment.
Found a blog post that explains that there are differences, my understanding is that the -m is slightly preferred in this case, but I'm not 100% sure still.
https://blog.ganssle.io/articles/2019/08/test-as-installed.html
There was a problem hiding this comment.
Interesting. I then do not have any preference. Happy to merge if you feel -m pytest ir preferable.
There was a problem hiding this comment.
Re-reading the post, looks like it might be better without the -m, I'll remove this change
Specify pythonpath in config file
|
This change causes a warning print in pytest versions < 7.0. We might want to update our requirements to be a version above 7.0. |
I got import errors due to running on my system version of xdsl as opposed to the local version. Specifying the
pythonpathin the config lets pytest choose the correct xdsl. I also addedpython -m pytestsince that's less likely to use a different python version to the one used to install dependencies.