New functions git.GetUserName and git.GetUserEmail #1453
Conversation
k8s-ci-robot
added
release-note
k8s-ci-robot
added
the
sig/release
| @@ -64,6 +67,84 @@ func TestGetDefaultKubernetesRepoURLSuccess(t *testing.T) { | |||
| } | |||
| } | |||
|
|
|||
| // createTestRepository creates a test repo, cd into it and returns the path | |||
| func createTestRepository() (repoPath string, err error) { | |||
There was a problem hiding this comment.
can newSUT() be reused here?
There was a problem hiding this comment.
I considered it, but I figured that instead of looking into only simulating the call by regenerating the fakes with counterfeiter (and possibly getting myself into a mess) it would be better to do a function that actually calls git in a repo. Do you think I should go with the simulated tests? (can do, no problem :)
There was a problem hiding this comment.
Ok I've delved a bit into this. Currently, there is no way to use newSUT() as the function only creates a mock Repository interface for the tests and most of the calls to git call the command package directly.
I was thinking about how to add the command fakes to the git package following @saschagrunert 's advice and we would have to something like this:
- Add a new interface to interface (duh) with the command package.
CommandCalleror something like that. - Replace all calls to
command.*in the git package to go through this interface. - Create the mock CommandCaller in the tests and then rewrite the tests with it.
What do you think @hasheddan @saschagrunert ? If this sounds good to you, I can rework the package later this week.
|
This PR may require API review. If so, when the changes are ready, complete the pre-review checklist and request an API review. Status of requested reviews is tracked in the API Review project. |
Signed-off-by: Adolfo García Veytia <[email protected]>
puerco
force-pushed
the
git-userdata-funcs
branch
from
e43f46a to
ec05406
Compare
|
/test pull-release-test |
There was a problem hiding this comment.
/lgtm
/hold
For the discussion https://github.com/kubernetes/release/pull/1453/files#r463996463
Feel free to lift the hold if we wanna follow-up on this one.
k8s-ci-robot
added
the
do-not-merge/hold
k8s-ci-robot
added
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: puerco, saschagrunert The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
OK, in order to make the tests all similar I'll start looking into creating a mock of the Command calls in the near future. Meanwhile in it goes. |
k8s-ci-robot
removed
the
do-not-merge/hold
What type of PR is this?
/kind api-change
/kind feature
What this PR does / why we need it:
This PR introduces two new functions to the got package:
git.GetUserNameandgit.GetUserEmailto be able to get the user's information outside of the git package.Tests for both functions are included in the git_test.go file
Which issue(s) this PR fixes:
Special notes for your reviewer:
Test for the functions will create an empty git repository, configure it with the user's data and call the functions in the git package.
Does this PR introduce a user-facing change?
Signed-off-by: Adolfo García Veytia [email protected]