Environment variable-related test fixes #623
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some unittests set environment variables, but then delete them as part of their cleanup process. Deleting them is OK. Any test that needs an environment variable should set that environment variable itself. Once the test process stops, any changes made to the environment will be lost [1]. Before this change, there was one location where an environment variable was restored to its original value instead of deleted. Restoring that variable was unnecessary. This commit was created to prepare for a future commit which will delete HOME before any of the tests even start. Without this change, that future change would crash. You can’t restore a variable that’s been deleted. Fixes adrienverge#605. [1]: <https://stackoverflow.com/q/716011/7593853>
Several environment variables can influence which config file yamllint chooses to use [1]. Before this change, if you set those environment variables and ran “python -m unittest discover”, then you could cause certain tests to use the wrong config file and fail. Fixes adrienverge#621. [1]: <https://github.com/adrienverge/yamllint/blob/152ba20f339588a872777eafb888f0073e83cafb/yamllint/cli.py#L180-L188>
Before this change one of yamllint’s tests wasn’t being run on GitHub Actions Runners because the HOME environment variable couldn’t be overridden. I just tested it, and it looks like the HOME variable can be overridden now, so that test no longer needs to be skipped.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See the individual commits for details.
While I was working on implementing what was discussed here, I realized that there were other edge cases where environment variables could cause problems. For example, running
YAMLLINT_CONFIG_FILE=/random/binary/file python -m pytest discovercaused a bunch of new test failures. I decided to implement a more robust solution that fixes the problem for more than justtest_run_with_user_global_config_file.Fixes #605. Fixes #621.