-
Notifications
You must be signed in to change notification settings - Fork 35
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
Add ability to use mock without passing in arguments #191
Changes from 2 commits
6d21f37
0f5ade0
b1f8465
c1944d6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,9 +49,20 @@ | |
|
||
var mockGitUrl util.MockGitUrl | ||
|
||
if util.IsGitProviderRepo(gc.MockGitURL.Host) { | ||
mockGitUrl = gc.MockGitURL | ||
mockGitUrl.Token = gc.GitTestToken | ||
if gc.MockGitURL.Host != "" { | ||
if util.IsGitProviderRepo(gc.MockGitURL.Host) { | ||
mockGitUrl = gc.MockGitURL | ||
mockGitUrl.Token = gc.GitTestToken | ||
} | ||
} else if params.URL != "" { | ||
// Not all clients have the ability to pass in mock data | ||
// So we should be adaptable and use the function params | ||
// and mock the output | ||
if util.IsGitProviderRepo(params.URL) { | ||
gc.MockGitURL.Host = params.URL | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for this case, is it possible that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Its not a pointer, I dont think we should check for nil here. Its a regular struct assignment. |
||
mockGitUrl = gc.MockGitURL | ||
mockGitUrl.Token = params.Token | ||
} | ||
} | ||
|
||
return mockGitUrl.DownloadInMemoryWithClient(params, httpClient, gc.DownloadOptions) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -90,8 +90,8 @@ func TestDownloadInMemoryClient(t *testing.T) { | |
}, | ||
{ | ||
name: "Case 6: Input url is valid with a mock client", | ||
client: MockDevfileUtilsClient{MockGitURL: util.MockGitUrl{Host: "https://github.com/devfile/library/blob/main/devfile.yaml"}, DownloadOptions: util.MockDownloadOptions{MockFile: "OK"}}, | ||
url: "https://github.com/devfile/library/blob/main/devfile.yaml", | ||
client: MockDevfileUtilsClient{MockGitURL: util.MockGitUrl{Host: server.URL}, DownloadOptions: util.MockDownloadOptions{MockFile: "OK"}}, | ||
url: server.URL, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are we able to test a gitURL?
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
Sorry, something went wrong. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since you add support for empty client, should add new unit test for your change, not editing the existing tests. the empty client case you added, needs a gitURL? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll add a Github URL |
||
want: []byte{79, 75}, | ||
}, | ||
{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add a test case for this block?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch, tests updated.