Skip to content

Conversation

@bmerkle
Copy link
Contributor

@bmerkle bmerkle commented Jan 1, 2026

a possible solution for #146

for details/question of implementation approach, please check #146

…d usage:

- pytest fixture (adding code and info to conftest.py)
- compute the testdata_path in only one place
- provide used test files as constants
…d usage:

- to non test code we shall discuss wether testdata makes sense at all
- here i have implemented a small workaround with reusage of conftest.py via a wrapper
@gvanrossum
Copy link
Collaborator

gvanrossum commented Jan 1, 2026

  • I like what you did for conftest and the tests.
  • I just realized that the tests are now in tests/test/test_foo.py. I had meant them to be in tests/test_foo.py (no "test/" folder in between). Sorry for not catching this in review.
  • The shenanigans needed to import things from tests/test/conftest.py or from tools/util_testdata.py are horrible (and worse because isort doesn't understand '# fmt: off'), so let's not do that.
  • I think maybe server.py and the tools should not have default paths built in at all. These will have to be passed in explicitly when we want to use them -- or they will have to continue to be hardcoded.

@bmerkle
Copy link
Contributor Author

bmerkle commented Jan 2, 2026

  • I just realized that the tests are now in tests/test/test_foo.py. I had meant them to be in tests/test_foo.py (no "test/" folder in between). Sorry for not catching this in review.

oops, they should not be in extra subdir. Sorry, my fault. It meant to be /tests only.
I will fix it. Seperate PR or in this PR ? i would prefer to fix that path issue in this PR directly

  • The shenanigans needed to import things from tests/test/conftest.py or from tools/util_testdata.py are horrible (and worse because isort doesn't understand '# fmt: off'), so let's not do that.

yes. I always feel bad when i do suppressions to hide hacks or workarounds like that. Will remove it.

  • I think maybe server.py and the tools should not have default paths built in at all. These will have to be passed in explicitly when we want to use them -- or they will have to continue to be hardcoded.

passing explicit is a good idea, let me try that. (explicit is better than implicit ;-)

- removed dependency on tests aka conftest.py
- added explicit arguments for  passing in testdata and validation logic
- adopted conftest.py to use only parent instaed of parent.parent
…t the subprocess needs the full environment (PATH, PYTHONPATH, system libraries, etc.) to import Python modules correctly.

Fix: Changed the fixture to inherit the full parent environment (dict(os.environ)) instead of building a minimal one.
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