Skip to content
This repository has been archived by the owner on Apr 24, 2020. It is now read-only.

[Bugfix] Don't unset global git user config after git tests #1221

Merged
merged 2 commits into from
Apr 1, 2019

Conversation

Syphdias
Copy link
Member

@Syphdias Syphdias commented Mar 19, 2019

Currently running the git unit tests unsets name and email in the global git config.

This PR gets rid of unnecessary variable juggling and simply sets the git config in the new home. The config (/tmp/powerlevel9k-test/.gitconfig) will be removed when the test directory is removed. I also added a "test" (which is a bit of a hack) to check if email or name were changed.

PS: Teardowns are currently called an additional time (see kward/shunit2#112 for details) after shunit finished all tests. It should be fine for now. Just a heads up since I stumbled upon this bug while testing.

also adds tests to check for tampering with the user config
@Syphdias Syphdias requested a review from dritter March 19, 2019 23:10
Copy link
Member

@dritter dritter left a comment

Choose a reason for hiding this comment

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

Overall I like the Idea of cleaning that up, but I have a bit different implementation in mind. This fiddling around with the git config was done, because we didn't had a better way to set a temporary configuration for the test. Later on, somebody else did just redeclare the HOME directory for a test. I think we could do the same here: Just set $HOME to a temporary directory, and write a ~/.gitconfig there. That should work as well, and we won't have to figure out the original git settings at first and reset them afterwards..

@Syphdias
Copy link
Member Author

@dritter I'm confused. How is that not what is being done here? Which is why I deleted most of the setting resets.

I also added the test to ensure the user config modification doesn't happen again. This apparently failed on Mac OS...

@dritter
Copy link
Member

dritter commented Mar 31, 2019

Ha! I missed that line (folded away by github).

So I adjust my comment: I wouldn't trust git config --global, and instead just write the config file directly to our newly created HOME folder.. ;)

@dritter dritter merged commit 67b4474 into Powerlevel9k:next Apr 1, 2019
@dritter
Copy link
Member

dritter commented Apr 1, 2019

Thanks @Syphdias .

@Syphdias Syphdias deleted the fix-git-test branch April 1, 2019 16:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants