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

test(monitor): mock file write for recovery tests #1599

Merged
merged 1 commit into from
Feb 14, 2025

Conversation

wyardley
Copy link
Contributor

@wyardley wyardley commented Feb 9, 2025

Fixes #1356
The tests for did_recovery and did_recovered touch a file that they leave around.
While we could use a temporary file, in this context, it seems like mocking subprocess.Popen should be sufficient.

Fixes jamesoff#1356
The tests for `did_recovery` and `did_recovered` touch a file that they
leave around.
While we could use a temporary file, in this context, it seems like
mocking subprocess.Popen should be sufficient.
@wyardley wyardley force-pushed the wyardley/issue_1356 branch from 74133e5 to 846597a Compare February 9, 2025 07:11
# throws an exception if the file isn't there
os.stat("did_recovery")
# Make sure we called the fake command
mock_popen.assert_called_once_with(["touch", "did_recovery"])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note: I kept the command and the arguments as they were. But since we're not calling it, this really could be anything (and could use more complex args if we want to make sure argument parsing / etc. is handled as expected.

@jamesoff jamesoff merged commit 2e14188 into jamesoff:develop Feb 14, 2025
21 checks passed
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.

Unit tests: clean up did_recovery and did_recovered
2 participants