added test files for profile and profile/status command by DharunMR · Pull Request #6420 · mindersec/minder

DharunMR · 2026-04-27T01:21:47Z

Description

This PR introduces a comprehensive refactor of the cmd/cli/app/profile and cmd/cli/app/profile/status package to modernize its testing architecture, improve separation of concerns. Stripped out manual ctx and grpc.ClientConn arguments from command execution functions to align with standard Cobra RunE signatures. Implemented the "backpack" pattern using cmd.SetContext(ctx) and cli.WithRPCClient to inject mock clients cleanly during testing.
Modified the apply command to accept and process multiple YAML file paths

🧪 How to Test

Run standard tests: go test ./... (Should PASS)
Run test if any changes made in cmd code then add -update flag: go test ./cmd/cli/app/profile/ -update and go test ./cmd/cli/app/profile/status -update

coveralls · 2026-04-27T03:32:34Z

Coverage is 60.574% — DharunMR:test-profile-cmd into mindersec:main. No base build found for mindersec:main.

evankanderson · 2026-04-27T20:38:02Z

+// GetDataSourceClient returns the DataSourceServiceClient, a cleanup function to close the connection and an error
+func GetDataSourceClient(cmd *cobra.Command) (minderv1.DataSourceServiceClient, func(), error) {
+	ctx, cancel := cli.GetAppContext(cmd.Context(), viper.GetViper())
+	cmd.SetContext(ctx)
+
+	if mockClient, ok := cli.GetRPCClient[minderv1.DataSourceServiceClient](ctx); ok {
+		return mockClient, func() { cancel() }, nil
+	}
+
+	conn, err := cli.GrpcForCommand(cmd, viper.GetViper())
+	if err != nil {
+		cancel()
+		return nil, nil, err
+	}
+
+	return minderv1.NewDataSourceServiceClient(conn), func() {
+		cancel()
+		_ = conn.Close()
+	}, nil
+}


I'm noticing that these seem very repetitive; can we use a generic version?

Suggested change

// GetDataSourceClient returns the DataSourceServiceClient, a cleanup function to close the connection and an error

func GetDataSourceClient(cmd *cobra.Command) (minderv1.DataSourceServiceClient, func(), error) {

ctx, cancel := cli.GetAppContext(cmd.Context(), viper.GetViper())

cmd.SetContext(ctx)

if mockClient, ok := cli.GetRPCClient[minderv1.DataSourceServiceClient](ctx); ok {

return mockClient, func() { cancel() }, nil

}

conn, err := cli.GrpcForCommand(cmd, viper.GetViper())

if err != nil {

cancel()

return nil, nil, err

}

return minderv1.NewDataSourceServiceClient(conn), func() {

cancel()

_ = conn.Close()

}, nil

}

type Cleanup = func()

// GetCLIClient takes a factory for a GRPC client service and returns

// a client, a cleanup function to close the connection and an error

func GetCLIClient[T interface](cmd *cobra.Command, client func(grpc.ClientConnInterface) T) (T, Cleanup, error) {

ctx, cancel := cli.GetAppContext(cmd.Context(), viper.GetViper())

cmd.SetContext(ctx)

if mockClient, ok := cli.GetRPCClient[T](ctx); ok {

return mockClient, func() { cancel() }, nil

}

conn, err := cli.GrpcForCommand(cmd, viper.GetViper())

if err != nil {

cancel()

return nil, nil, err

}

return client(conn), func() {

cancel()

_ = conn.Close()

}, nil

}

evankanderson · 2026-04-27T20:40:43Z


 type protoWithProfileStatus interface {
-	proto.Message
+	protoreflect.ProtoMessage


we should prefer proto.Message, but a bunch of early usage (before my arrival) used the protoreflect package, and it got copied lots of places.

Suggested change

protoreflect.ProtoMessage

proto.Message

evankanderson · 2026-04-27T20:46:28Z

This looks terrible (yaml wrapped, etc), but it is what it is. Maybe file an issue that "Rule definition" is not very helpful in the current apply output?

(Thanks for doing this, btw! It makes it much easier to see where the formatting sucks)

Yea i'll file an issue

evankanderson · 2026-04-27T20:48:12Z

+Successfully created new profile named: mock-attestation-profile
+Successfully updated existing profile named: mock-dependabot-profile
+ ENTITY     │ RULE                      │ RULE PARAMS              │ RULE DEFINITION                


Interesting... if one of these profiles has two rules, what does the output look like? It feels like we might need a delimiter (extra column wouldn't fit) for the profile name.

i made table to render after every success profile creation , does it look better now?

Modulo https://github.com/mindersec/minder/pull/6420/changes#r3150092088, yes.

Thanks for doing this -- I'm inclined to defer output cleanup on the CLI until after we have the existing output captured by these tests (but maybe we file issues for the cleanup).

evankanderson · 2026-04-27T20:49:44Z

+name: mock-profile
+repository:
+    - type: mock-rule-type
+type: profile
+version: v1
+---
+def:
+    eval:
+        data_sources:
+            - name: mock-data-source
+name: mock-rule-type
+type: rule-type
+version: v1
+---
+name: mock-data-source
+type: data-source
+version: v1


This output is weird -- can we fill in enough fields that it would make sense? (It's also annoying that the field sort "feels" random when it's alphabetical rather than by some other canonical ordering ala Kubernetes.)

yes the alphabetical sort makes these hard to parse visually, i think i can open a follow-up pr for canonical ordering of fields

evankanderson · 2026-04-27T20:57:34Z

+	}
+	defer closeRules()

 	// TODO: it would be nice if this were just a list of rules...


I think this TODO may be done?

TODO actually got misplaced during a refactor. i think its still unresolved because the protobuf schema still partitions rules by entity type?

Signed-off-by: DharunMR <maddharun56@gmail.com>

evankanderson

I'm going to merge this to un-block #6417 and others.

evankanderson · 2026-04-30T16:56:34Z

+
+ ENTITY                  │ RULE NAME            │ STATUS  │ DETAILS                                 
+─────────────────────────┼──────────────────────┼─────────┼─────────────────────────────────────────
+ acme-corp/mock-repo     │                      │ Ok      │ Mock rule evaluation succeeded.         


Empty rule name here is weird, but it seems like we're passing the tests, so this should be a follow up fix in coordination with #6417.

evankanderson · 2026-04-30T17:01:45Z

+Successfully created new profile named: mock-attestation-profile
+Successfully updated existing profile named: mock-dependabot-profile
+ ENTITY     │ RULE                      │ RULE PARAMS              │ RULE DEFINITION                


Modulo https://github.com/mindersec/minder/pull/6420/changes#r3150092088, yes.

Thanks for doing this -- I'm inclined to defer output cleanup on the CLI until after we have the existing output captured by these tests (but maybe we file issues for the cleanup).

evankanderson · 2026-04-30T17:02:37Z

+name: mock-profile
+repository:
+    - type: mock-rule-type
+type: profile
+version: v1
+---
+def:
+    eval:
+        data_sources:
+            - name: mock-data-source
+name: mock-rule-type
+type: rule-type
+version: v1
+---
+name: mock-data-source
+type: data-source
+version: v1


evankanderson · 2026-04-30T17:11:02Z

+	applyCmd.Flags().StringArrayP("file", "f", []string{},
+		"Path to the YAML defining the profile (or - for stdin). Can be specified multiple files")


It's okay this time, but let's try to separate behavior changes from e.g. test and refactoring changes that don't change behavior.

evankanderson · 2026-04-30T17:14:46Z

+	var datasources []string
+	for _, rtName := range ruletypes {


This refactor makes it harder to review the PR because I now have to start a second context (what's going on with these functions?) in addition to the rule output changes. While it's super-tempting to fix things as you see them, it's easier to review 2 focused PRs than one PR that mixes behavior. (Renaming all the ruletype variables also introduces a bunch of extra diffs.)

sure i will follw it

evankanderson · 2026-04-30T17:19:10Z

-			lw := utf8.RuneCountInString(line)
+			lw := text.RuneWidthWithoutEscSequences(line)


Ah, was this a bug?

yea, this printing color and emoji as code instead as it is

DharunMR requested a review from a team as a code owner April 27, 2026 01:21

DharunMR force-pushed the test-profile-cmd branch 5 times, most recently from 70bb101 to 6eeeedc Compare April 27, 2026 03:21

evankanderson reviewed Apr 27, 2026

View reviewed changes

evankanderson mentioned this pull request Apr 27, 2026

CLI : added unit test files for project and project role #6418

Open

DharunMR force-pushed the test-profile-cmd branch from 6eeeedc to 0884068 Compare April 28, 2026 06:02

added test files for profile and profile/status command

ffeb4b9

Signed-off-by: DharunMR <maddharun56@gmail.com>

DharunMR force-pushed the test-profile-cmd branch from 0884068 to ffeb4b9 Compare April 28, 2026 06:06

DharunMR requested a review from evankanderson April 28, 2026 06:20

sachin9058 mentioned this pull request Apr 30, 2026

test(cli): add artifact CLI tests using mock-based golden framework #6419

Open

evankanderson approved these changes Apr 30, 2026

View reviewed changes

evankanderson merged commit a1a8103 into mindersec:main Apr 30, 2026
26 checks passed

evankanderson mentioned this pull request Apr 30, 2026

feat(cli): improve evaluation reasoning output using existing fields #6417

Merged

sachin9058 mentioned this pull request Apr 30, 2026

Unstable JSON output in CLI tests due to protojson formatting #6430

Open

This was referenced Jul 16, 2026

minder 0.2.0 Homebrew/homebrew-core#293608

Merged

minder 0.2.2 Homebrew/homebrew-core#294363

Merged

		applyCmd.Flags().StringArrayP("file", "f", []string{},
		"Path to the YAML defining the profile (or - for stdin). Can be specified multiple files")

		lw := utf8.RuneCountInString(line)
		lw := text.RuneWidthWithoutEscSequences(line)

Uh oh!

Conversation

DharunMR commented Apr 27, 2026

Description

🧪 How to Test

Uh oh!

coveralls commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

evankanderson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coveralls commented Apr 27, 2026 •

edited

Loading