KEMBAR78
Deleted some temporary files that CLI tests were creating by smsearcy · Pull Request #153 · theskumar/python-dotenv · GitHub
Skip to content

Conversation

@smsearcy
Copy link
Contributor

While working on another PR I noticed that running the test suite was leaving a tests/dotenv file. There appeared to be two test functions that were creating that file but not deleting it when done.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 89.338% when pulling 2939cb9 on smsearcy:test-cleanup into 3b7e60e on theskumar:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 89.338% when pulling 2939cb9 on smsearcy:test-cleanup into 3b7e60e on theskumar:master.

@theskumar theskumar force-pushed the master branch 2 times, most recently from f628767 to f9863d3 Compare December 16, 2018 13:18
@bbc2
Copy link
Collaborator

bbc2 commented Dec 30, 2018

This looks useful. Thanks for improving test isolation!

I realize these tests are really fishy, notably because they use cli.isolated_filesystem and then change the current directory to somewhere outside that filesystem. I might later look into refactoring that to prevent such issues from happening again.

@bbc2 bbc2 merged commit cf9bd1f into theskumar:master Dec 30, 2018
bbc2 added a commit to bbc2/python-dotenv that referenced this pull request Dec 31, 2018
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".
@bbc2 bbc2 mentioned this pull request Dec 31, 2018
theskumar pushed a commit that referenced this pull request Jan 1, 2019
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 #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".
johnbergvall pushed a commit to johnbergvall/python-dotenv that referenced this pull request Aug 13, 2021
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".
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.

3 participants