Add Release Bundle Evidence Using Cli Client #14
Conversation
|
CLA Assistant Lite bot: I have read the CLA Document and I hereby sign the CLA 1 out of 2 committers have signed the CLA. |
|
I have read the CLA Document and I hereby sign the CLA |
evidence/cli/command_controller.go
Outdated
| if subject == releaseBundle { | ||
| command = NewEvidenceReleaseBundleCommand(c) | ||
| } | ||
| return command.CreateEvidence(artifactoryClient) |
There was a problem hiding this comment.
Unsafe code, the command may be nil.
evidence/cli/command_controller.go
Outdated
| } | ||
| platformToEvidenceUrls(artifactoryClient) | ||
| return artifactoryClient, nil | ||
| if subject == releaseBundle { |
There was a problem hiding this comment.
Should be either a switch or if/else (no need to check for if after the match).
evidence/cli/command_controller.go
Outdated
| @@ -78,16 +62,51 @@ func validateCreateEvidenceContext(c *components.Context) error { | |||
| if !c.IsFlagSet(EvdPredicateType) || assertValueProvided(c, EvdPredicateType) != nil { | |||
| return errorutils.CheckErrorf("'predicate' is a mandatory field for creating a custom evidence: --%s", EvdPredicateType) | |||
There was a problem hiding this comment.
Typo: predicate-type instead of predicate in the message.
evidence/cli/command_controller.go
Outdated
| if !c.IsFlagSet(EvdKey) || assertValueProvided(c, EvdKey) != nil { | ||
| return errorutils.CheckErrorf("'key' is a mandatory field for creating a custom evidence: --%s", EvdKey) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func getAndValidateSubject(c *components.Context) (string, error) { | ||
| subjects := []string{ |
There was a problem hiding this comment.
Can be moved outside of the function (no need to re-allocate each time).,
| if tt.expectErr { | ||
| assert.Error(t, err) | ||
| } else { | ||
| assert.NoError(t, err) |
There was a problem hiding this comment.
The NoError is not reachable (all cases are negative, please add one with expectErr: false)
evidence/cli/command_custom.go
Outdated
| coreConfig "github.com/jfrog/jfrog-cli-core/v2/utils/config" | ||
| ) | ||
|
|
||
| type EvidenceCustomCommand struct { |
There was a problem hiding this comment.
Why is it an exported struct and not an interface?
| coreConfig "github.com/jfrog/jfrog-cli-core/v2/utils/config" | ||
| ) | ||
|
|
||
| type EvidenceReleaseBundleCommand struct { |
There was a problem hiding this comment.
Why is it an exported struct and not an interface?
| ctrl := gomock.NewController(t) | ||
| defer ctrl.Finish() | ||
|
|
||
| mockExec := func(command commands.Command) error { |
There was a problem hiding this comment.
The override can be applied once for all the test cases:
execFunc = func(command commands.Command) error {
return nil
}(no need to repeat the same mockExec and defer the restoration.
evidence/cli/command_controller.go
Outdated
| rtDetails.ArtifactoryUrl = utils.AddTrailingSlashIfNeeded(rtDetails.Url) + "artifactory/" | ||
| rtDetails.EvidenceUrl = utils.AddTrailingSlashIfNeeded(rtDetails.Url) + "evidence/" | ||
| } | ||
| var execFunc = exec |
There was a problem hiding this comment.
As the exec function is used only from this variable, it can be inlined like:
var execFunc = func(command commands.Command) error {
return commands.Exec(command)
}
evidence/cli/utils.go
Outdated
| } | ||
|
|
||
| var subjectTypes = []string{ | ||
| EvdRepoPath, |
There was a problem hiding this comment.
The EvdRepoPath can be non-exported (the same as releaseBundle).
Also, the type name is quite confusing, is it subjectRepoPath or just repoPath?
(we do not specify a path to the evidence itself)
evidence/cli/mocks/command_mock.go
Outdated
There was a problem hiding this comment.
Please, do not commit mocks.
Need to add *_mock.go to the .gitignore.
evidence/create_base.go
Outdated
| log.Warn(fmt.Sprintf("failed to read predicate file '%s'", predicate)) | ||
| return nil, err | ||
| } | ||
| //client.NewClient(c.serverDetails) |
There was a problem hiding this comment.
Please, remove the commented-out code.
No description provided.