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

Fix chdir leaking on error out of cd-contextmanager of tests/helper.py. #3720

Merged
merged 1 commit into from
Nov 14, 2023

Conversation

aknrdureegaesr
Copy link
Contributor

@aknrdureegaesr aknrdureegaesr commented Nov 14, 2023

Pull Request Checklist

  • I’ve read the guidelines for contributing.
  • I updated AUTHORS.txt and CHANGES.txt (if the change is non-trivial) and documentation (if applicable).
  • I tested my changes.

Description

While working on something else, I happened to spot a bug in tests/helper.py: The change of directory can (unintentionally) leak out of the cd context manager if the code inside the context throws an exception. In other words, on error, the change of directory is not undone.

I added a test for the test helper that demonstrates the bug if run with the previous test helper code. Not sure whether you want that.

This is a rather trivial change not visible to the end-user, so there is no need to add to CHANGELOG.

I plead guilty not having created a separate bug issue first. Do you need it in such a trivial case?

@Kwpolska Kwpolska merged commit 8689cf3 into getnikola:master Nov 14, 2023
10 checks passed
@Kwpolska
Copy link
Member

I plead guilty not having created a separate bug issue first. Do you need it in such a trivial case?

Not really.

Thanks for fixing this!

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