Add 'sshkey' context manager #242
Conversation
| self.set_environment(**old_env) | ||
|
|
||
| @contextmanager | ||
| def sshkey(self, sshkey_file): |
There was a problem hiding this comment.
For a moment I thought this method is to specific, but on the other hand, it makes setting ssh-keys (by using key-files) really easy.
Therefore the only thing I will do is see how this works on windows, and possibly put in an assertion to run on non-windows systems only.
|
Thank you ! |
| @@ -153,3 +153,23 @@ def test_env_vars_passed_to_git(self): | |||
| editor = 'non_existant_editor' | |||
| with mock.patch.dict('os.environ', {'GIT_EDITOR': editor}): | |||
| assert self.git.var("GIT_EDITOR") == editor | |||
|
|
|||
| def test_environment(self): | |||
There was a problem hiding this comment.
I don't see how I could test the sshkey context manager without accessing an actual repo. Also, how could I check whether the environment is actually passed into the git process? The former would cover the latter. Do you have ideas?
There was a problem hiding this comment.
Something that could work is to adjust the script for the test to actually fail with a distinct error code, and possibly emit something distinctive to STDERR.
That way, you could call the real git process, on a bogus git@server:foo/bar repo URL, and verify that it failed under your conditions. That can proof that the script was actually called.
The cool thing is that this would require you to put in a way to adjust the path the script handling the GIT_SSH, which could be done so that advanced users can either set it, or derive from the Git command implementation.
|
You did it ! Note to self: Test this on windows and at least throw saying sshkey doesn't work, if it doesn't work Also I believe today will be the day, as I should get through my docs review, and prepare a new release. |
|
Before I forget: I had to remove |
|
Thanks for the note, I already saw it! :) I didn't realise it would be such a pain! I'll stick with my local SSH script then in the meantime. :-) Thanks for merging everything else in! |
Proposed implementation for #234
I have done some manual tests, but not set up automated tests yet. I will see how it fits into your suite tomorrow if you haven't implemented it yet. :)
Let me know if something's not quite in the right place.