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 init files to test folder #136

Merged

Conversation

Newtoniano
Copy link
Contributor

Closes #135

Copy link
Owner

@diogomatoschaves diogomatoschaves left a comment

Choose a reason for hiding this comment

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

For the sake of consistency, maybe also add to the data/tests/service folder? Also, do execution/tests/service/cron_jobs and execution/tests/service/helpers also need this?

@Newtoniano
Copy link
Contributor Author

Newtoniano commented Jan 21, 2024

For the bug specifically, we need these in the exact folders where there is a file with an identical name to another test file that resides somewhere else in the repo. See pytest-dev/pytest#3151 .

Adding them in all folders shouldn't be necessary and isn't recommended by pytest because it this can change the way that pytest discovers and imports the test files, which can lead to unexpected behavior. I tried experimenting the fix suggested here pytest-dev/pytest#3151 (comment) but without success, I actually get even more import errors and modules not found like that. You can go down a rabbit hole of very confused developers that either suggest adding or removing __init__.py files without a clear explanation on how this import failure is happening.

Honestly, all these things are headaches that I never quite understood fully, and I also think pytest imports modules in a slightly different way from python, I just did the bare minimum needed to import all the tests correctly, but if in the future you're gonna create files with identical names across the microservices or even within the same service, then you might need to add a few more of these init files. If you run tests for each folder separately via cli, nothing should be needed anyway.

The safest approach is probably not to have test files with identical names across the microservices, but since many things are duplicated by design, one might forget.

@Newtoniano
Copy link
Contributor Author

Do you want to rename the conflicting files to something like test_model_app.py and test_execution_app.py instead? It solves the problem and it's probably a cleaner solution

@diogomatoschaves
Copy link
Owner

Yeah that is probably a cleaner solution. Can you rename the corresponding data file to test_data_app? In order to keep consistency.

@Newtoniano
Copy link
Contributor Author

Newtoniano commented Jan 22, 2024

Done, you might want to adopt this schema in the future for all test files so that there is never gonna be any conflict.

@diogomatoschaves
Copy link
Owner

Nice one 👍 Can you just squash all those commits? As they basically create and then delete the same files.

@Newtoniano Newtoniano force-pushed the bugfix/pytest-imports branch from 7c14d9b to e5d96b6 Compare January 22, 2024 14:51
@Newtoniano
Copy link
Contributor Author

should be good now

@diogomatoschaves diogomatoschaves merged commit 1db099d into diogomatoschaves:master Jan 22, 2024
@Newtoniano Newtoniano deleted the bugfix/pytest-imports branch January 22, 2024 15:46
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.

Pytest is unable to import some tests
2 participants