-
Notifications
You must be signed in to change notification settings - Fork 70
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
CLOUDP-78654: Missing audit filter on convert #545
Conversation
@@ -173,7 +173,7 @@ func createProject(projectName string) (string, error) { | |||
cmd.Env = os.Environ() | |||
resp, err := cmd.CombinedOutput() | |||
if err != nil { | |||
return "", err | |||
return "", fmt.Errorf("%s (%w)", string(resp), err) |
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.
drive by debug, a tests was failing here and needed to see why
@@ -363,6 +363,7 @@ func (p *ProcessConfig) auditLog() *opsmngr.AuditLog { | |||
Destination: p.auditLogDestination(), | |||
Path: p.AuditLogPath, | |||
Format: p.AuditLogFormat, | |||
Filter: p.AuditLogFilter, |
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.
I missed this, the rest is tests
@@ -116,3 +113,166 @@ func TestNewReplicaSetProcessConfig(t *testing.T) { | |||
|
|||
assert.Equal(t, expected, result) | |||
} | |||
|
|||
func Test_newConfigRSProcess(t *testing.T) { |
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.
I added a test of ops manager -> mcli but not one mcli -> ops manager, so now we have both
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.
LGTM!
Proposed changes
Jira ticket: CLOUDP-78654
Checklist
make fmt
and formatted my codeFurther comments