-
Notifications
You must be signed in to change notification settings - Fork 478
Isolate test files #160
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
Isolate test files #160
Conversation
The test suite had the following issues: * A failing test would leave files in the `tests` directory, causing other tests to fail unexpectedly. * Even successful tests would require clean up code like `sh.rm(...)` to avoid leaving test files. This was easy to forget as shown by theskumar#153. * The same test file would be used for several tests, making them unsafe to run in parallel. This commit fixes that by using the `dotenv_file` and `tmp_path` fixtures whenever the filesystem is required. The advantage of those fixtures is that they use unique test directories, which are eventually cleaned up. The `tmp_path` fixture is preferred to the `tmpdir` fixture because `tmpdir` relies on `py.path`, a library in "maintenance mode".
| assert os.environ[key_name] == 'WORKS' | ||
|
|
||
|
|
||
| @pytest.mark.xfail(sys.version_info < (3, 0), reason="test was incomplete") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's odd is that reviewing the coverage changes for #152 after merging #154 isn't showing it covering that code, and yet now the test passes. I'll dig into this a little more.
ETA: Based on the coverage docs it appears that there are some subprocess/threading calls that it cannot track, I suspect that is what is happening here because I confirmed via some print statements that we are definitely hitting the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new test unfortunately doesn't use the same Python VM, and so doesn't contribute to coverage. But this is also what makes this test isolated and realistic.
1 similar comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! 💟
* master: (49 commits) Fix typo in README.md (#181) Refactor move run_command in cli Refractor: move 'to_env' to compat.py Refactor parser to fix inconsistencies (#180) chore(pypi): switch to markdown and twine for pypi upload + update supported version Bump version: 0.10.1 → 0.10.2 readme: Add new release stub Fix unicode/str inconsistency in Python 2 (#177) Add argument to choose .env file encoding, defaults to None Fix links in readme Restrict typing dependency to Python < 3.5 Add type hints and expose them to users Updated `.gitignore` with results from https://www.gitignore.io/api/python Added special case for `__file__` in Python 2. Fixes #130 Updated tests to show that this works. Moved `dotenv` package into `src` directory so that tests run against installed version. Added tox to run test suite against multiple versions of Python (with coverage) and run flake8. Updated Travis CI to use tox as well. Updated documentation about running tests. Fix ResourceWarning: unclosed file in setup.py and test_cli.py Clarify the usuages of export Isolate test files (#160) Deleted some temporary files that CLI tests were creating. Add python 3.7 to testsuite + udpate pypi creds ...
The test suite had the following issues: * A failing test would leave files in the `tests` directory, causing other tests to fail unexpectedly. * Even successful tests would require clean up code like `sh.rm(...)` to avoid leaving test files. This was easy to forget as shown by theskumar#153. * The same test file would be used for several tests, making them unsafe to run in parallel. This commit fixes that by using the `dotenv_file` and `tmp_path` fixtures whenever the filesystem is required. The advantage of those fixtures is that they use unique test directories, which are eventually cleaned up. The `tmp_path` fixture is preferred to the `tmpdir` fixture because `tmpdir` relies on `py.path`, a library in "maintenance mode".
The test suite had the following issues:
testsdirectory, causingother tests to fail unexpectedly.
sh.rm(...)toavoid leaving test files. This was easy to forget as shown by Deleted some temporary files that CLI tests were creating #153.
to run in parallel.
This commit fixes that by using the
dotenv_fileandtmp_pathfixtures whenever the filesystem is required. The advantage of those
fixtures is that they use unique test directories, which are eventually
cleaned up.
The
tmp_pathfixture is preferred to thetmpdirfixture becausetmpdirrelies onpy.path, a library in "maintenance mode".