From 8f4912491d6b499144538bda39d68e35fba2b5d7 Mon Sep 17 00:00:00 2001 From: krrish175-byte Date: Sat, 27 Jun 2026 17:07:00 +0530 Subject: [PATCH 1/8] feat: implement mindev test cli command and rule loading by name --- cmd/dev/app/root.go | 2 + cmd/dev/app/test/test.go | 59 +++++++++++++++++ pkg/ruletest/eval.go | 41 +++++++----- pkg/ruletest/eval_test.go | 3 +- pkg/ruletest/runner.go | 126 ++++++++++++++++++++++++++++++++---- pkg/ruletest/runner_test.go | 19 +++++- 6 files changed, 218 insertions(+), 32 deletions(-) create mode 100644 cmd/dev/app/test/test.go diff --git a/cmd/dev/app/root.go b/cmd/dev/app/root.go index f6cc36f9df..467fa087b8 100644 --- a/cmd/dev/app/root.go +++ b/cmd/dev/app/root.go @@ -11,6 +11,7 @@ import ( "github.com/mindersec/minder/cmd/dev/app/datasource" "github.com/mindersec/minder/cmd/dev/app/image" "github.com/mindersec/minder/cmd/dev/app/rule_type" + "github.com/mindersec/minder/cmd/dev/app/test" "github.com/mindersec/minder/cmd/dev/app/testserver" "github.com/mindersec/minder/internal/util/cli" ) @@ -26,6 +27,7 @@ https://mindersec.github.io/`, } cmd.AddCommand(rule_type.CmdRuleType()) + cmd.AddCommand(test.CmdTest()) cmd.AddCommand(image.CmdImage()) cmd.AddCommand(testserver.CmdTestServer()) cmd.AddCommand(bundles.CmdBundle()) diff --git a/cmd/dev/app/test/test.go b/cmd/dev/app/test/test.go new file mode 100644 index 0000000000..b09dfd5924 --- /dev/null +++ b/cmd/dev/app/test/test.go @@ -0,0 +1,59 @@ +// SPDX-FileCopyrightText: Copyright 2026 The Minder Authors +// SPDX-License-Identifier: Apache-2.0 + +// Package test provides the test command for mindev +package test + +import ( + "fmt" + + "github.com/spf13/cobra" + + "github.com/mindersec/minder/pkg/ruletest" +) + +// CmdTest returns the test cobra command +func CmdTest() *cobra.Command { + cmd := &cobra.Command{ + Use: "test [directories...]", + Short: "Run Minder rule tests", + Long: `Run Starlark-based tests for Minder rules. If no directories are provided, tests the current directory.`, + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) == 0 { + args = []string{"."} + } + + runner := ruletest.NewRunner() + results, err := runner.RunPaths(args) + if err != nil { + cmd.PrintErrf("Error(s) running tests:\n%v\n", err) + } + + if len(results) == 0 { + cmd.Printf("No tests found\n") + return err + } + + hasFailures := false + for _, res := range results { + if len(res.Failures) > 0 { + hasFailures = true + cmd.Printf("FAIL: %s\n", res.Name) + for _, f := range res.Failures { + cmd.Printf(" - %s\n", f) + } + } else { + cmd.Printf("PASS: %s\n", res.Name) + } + } + + if hasFailures { + return fmt.Errorf("one or more tests failed") + } + + return err + }, + } + + return cmd +} diff --git a/pkg/ruletest/eval.go b/pkg/ruletest/eval.go index bb359c3eaa..e402dc6e9b 100644 --- a/pkg/ruletest/eval.go +++ b/pkg/ruletest/eval.go @@ -21,36 +21,47 @@ import ( tkv1 "github.com/mindersec/minder/pkg/testkit/v1" ) -func builtinEval( +func (tr *testCaseRunner) builtinEval( thread *starlark.Thread, _ *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple, ) (starlark.Value, error) { - var rulePath string + var ruleNameOrPath string var entityDict *starlark.Dict var profileDict *starlark.Dict var mockHttpDict *starlark.Dict err := starlark.UnpackArgs("eval", args, kwargs, - "rule", &rulePath, "entity?", &entityDict, "profile?", &profileDict, "mock_http?", &mockHttpDict) + "rule", &ruleNameOrPath, "entity?", &entityDict, "profile?", &profileDict, "mock_http?", &mockHttpDict) if err != nil { return nil, err } - if !filepath.IsAbs(rulePath) { - callerFrame := thread.CallFrame(1) - if callerFile := callerFrame.Pos.Filename(); callerFile != "" { - rulePath = filepath.Join(filepath.Dir(callerFile), rulePath) + var rt *minderv1.RuleType + + if tr.ruleTypes != nil { + if ruleType, ok := tr.ruleTypes[ruleNameOrPath]; ok { + rt = ruleType } } - decoder, closer := fileconvert.DecoderForFile(rulePath) - if decoder == nil { - return nil, fmt.Errorf("error opening file: %s", rulePath) - } - defer closer.Close() + if rt == nil { + rulePath := ruleNameOrPath + if !filepath.IsAbs(rulePath) { + callerFrame := thread.CallFrame(1) + if callerFile := callerFrame.Pos.Filename(); callerFile != "" { + rulePath = filepath.Join(filepath.Dir(callerFile), rulePath) + } + } - rt, err := fileconvert.ReadResourceTyped[*minderv1.RuleType](decoder) - if err != nil { - return nil, fmt.Errorf("failed to parse rule type: %w", err) + decoder, closer := fileconvert.DecoderForFile(rulePath) + if decoder == nil { + return nil, fmt.Errorf("error opening file: %s (or rule not found in loaded rule types)", rulePath) + } + defer closer.Close() + + rt, err = fileconvert.ReadResourceTyped[*minderv1.RuleType](decoder) + if err != nil { + return nil, fmt.Errorf("failed to parse rule type: %w", err) + } } profileMap, err := dictToGoMap(profileDict) diff --git a/pkg/ruletest/eval_test.go b/pkg/ruletest/eval_test.go index 6752f62d56..f1798f1b72 100644 --- a/pkg/ruletest/eval_test.go +++ b/pkg/ruletest/eval_test.go @@ -100,7 +100,8 @@ func TestBuiltinEval_InvalidArgs(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - _, err := builtinEval(thread, starlark.NewBuiltin("eval", builtinEval), tt.args, tt.kwargs) + tr := &testCaseRunner{} + _, err := tr.builtinEval(thread, starlark.NewBuiltin("eval", tr.builtinEval), tt.args, tt.kwargs) if err == nil { t.Fatal("expected error, got nil") } diff --git a/pkg/ruletest/runner.go b/pkg/ruletest/runner.go index c21d69e3af..e4900ada3e 100644 --- a/pkg/ruletest/runner.go +++ b/pkg/ruletest/runner.go @@ -7,6 +7,7 @@ package ruletest import ( "errors" "fmt" + "io" "io/fs" "os" "path/filepath" @@ -14,6 +15,8 @@ import ( "strings" "testing" + minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" + "github.com/mindersec/minder/pkg/fileconvert" "go.starlark.net/starlark" "go.starlark.net/starlarktest" "go.starlark.net/syntax" @@ -26,15 +29,17 @@ type testCaseRunner struct { fs fs.FS predeclared starlark.StringDict failures []string + ruleTypes map[string]*minderv1.RuleType } -func (r *Runner) newTestCaseRunner(name string, fileSystem fs.FS) *testCaseRunner { +func (r *Runner) newTestCaseRunner(name string, fileSystem fs.FS, ruleTypes map[string]*minderv1.RuleType) *testCaseRunner { if fileSystem == nil { panic("fileSystem cannot be nil") } tr := &testCaseRunner{ fs: fileSystem, predeclared: starlark.StringDict{}, + ruleTypes: ruleTypes, } tr.thread = &starlark.Thread{ Name: name, @@ -42,7 +47,7 @@ func (r *Runner) newTestCaseRunner(name string, fileSystem fs.FS) *testCaseRunne } starlarktest.SetReporter(tr.thread, tr) - tr.predeclared["eval"] = starlark.NewBuiltin("eval", builtinEval) + tr.predeclared["eval"] = starlark.NewBuiltin("eval", tr.builtinEval) tr.predeclared["read_file"] = starlark.NewBuiltin("read_file", tr.builtinReadFile) tr.predeclared["txtar"] = starlark.NewBuiltin("txtar", builtinTxtar) tr.predeclared["body"] = starlark.NewBuiltin("body", builtinBody) @@ -90,10 +95,9 @@ func NewRunner() *Runner { } } -// RunFile executes a single Starlark test file and returns the results -// for each test_* function found in it. -// src may be nil, or a string, []byte, or io.Reader containing the file source. -func (r *Runner) RunFile(filename string, src any) ([]TestResult, error) { +// RunFile executes a single Starlark test file. If src is non-nil, it is +// used as the file contents. +func (r *Runner) RunFile(filename string, src any, ruleTypes map[string]*minderv1.RuleType) ([]TestResult, error) { if filename == "" { return nil, errors.New("filename cannot be empty") } @@ -102,11 +106,11 @@ func (r *Runner) RunFile(filename string, src any) ([]TestResult, error) { fileSystem := os.DirFS(baseDir) name := filepath.Base(filename) - tr := r.newTestCaseRunner(name, fileSystem) + tr := r.newTestCaseRunner(name, fileSystem, ruleTypes) globals, err := tr.runFile(filename, src) if err != nil { - if evalErr, ok := errors.AsType[*starlark.EvalError](err); ok { + if evalErr, ok := err.(*starlark.EvalError); ok { return nil, fmt.Errorf("loading %s: %w\n%s", filename, err, evalErr.Backtrace()) } return nil, fmt.Errorf("loading %s: %w", filename, err) @@ -129,15 +133,20 @@ func (r *Runner) RunFile(filename string, src any) ([]TestResult, error) { var results []TestResult for name, fn := range testFns { - result := r.runOneTest(name, fn, fileSystem) + result := r.runOneTest(name, fn, fileSystem, ruleTypes) results = append(results, result) } return results, nil } -func (r *Runner) runOneTest(name string, fn *starlark.Function, fileSystem fs.FS) TestResult { - tr := r.newTestCaseRunner(name, fileSystem) +func (r *Runner) runOneTest( + name string, + fn *starlark.Function, + fileSystem fs.FS, + ruleTypes map[string]*minderv1.RuleType, +) TestResult { + tr := r.newTestCaseRunner(name, fileSystem, ruleTypes) result := TestResult{Name: name} _, err := starlark.Call(tr.thread, fn, nil, nil) @@ -174,11 +183,100 @@ func DiscoverFiles(root string) ([]string, error) { return files, nil } +// loadRulesFromDir finds and parses all *.yaml files in the given directory +// into a map of RuleTypes keyed by rule name. +func loadRulesFromDir(dir string) (map[string]*minderv1.RuleType, error) { + ruleTypes := make(map[string]*minderv1.RuleType) + yamlFiles, err := filepath.Glob(filepath.Join(dir, "*.yaml")) + if err != nil { + return nil, fmt.Errorf("globbing yaml files: %w", err) + } + for _, yf := range yamlFiles { + decoder, closer := fileconvert.DecoderForFile(yf) + if decoder == nil { + return nil, fmt.Errorf("error opening file: %s", yf) + } + defer func(c io.Closer) { + _ = c.Close() + }(closer) + rt, err := fileconvert.ReadResourceTyped[*minderv1.RuleType](decoder) + if err == nil && rt != nil && rt.Name != "" { + ruleTypes[rt.Name] = rt + } + } + return ruleTypes, nil +} + // RunDir discovers and executes all *.star test files under the given -// directory, reporting results through t. -func (r *Runner) RunDir(t *testing.T, dir string) { +// directory. It also discovers and loads any *.yaml rule files in the directory. +func (r *Runner) RunDir(dir string) ([]TestResult, error) { + ruleTypes, err := loadRulesFromDir(dir) + if err != nil { + return nil, fmt.Errorf("loading rules: %w", err) + } + + files, err := DiscoverFiles(dir) + if err != nil { + return nil, fmt.Errorf("discovering test files: %w", err) + } + + var allResults []TestResult + for _, file := range files { + results, err := r.RunFile(file, nil, ruleTypes) + if err != nil { + return nil, err + } + allResults = append(allResults, results...) + } + + return allResults, nil +} + +// RunPaths takes a list of file or directory paths, executing tests in each. +// It collects errors instead of returning early on the first error. +func (r *Runner) RunPaths(paths []string) ([]TestResult, error) { + var allResults []TestResult + var errs []error + + for _, p := range paths { + info, err := os.Stat(p) + if err != nil { + errs = append(errs, fmt.Errorf("stat %s: %w", p, err)) + continue + } + if info.IsDir() { + res, err := r.RunDir(p) + if err != nil { + errs = append(errs, fmt.Errorf("error running directory %s: %w", p, err)) + } + allResults = append(allResults, res...) + } else { + ruleTypes, err := loadRulesFromDir(filepath.Dir(p)) + if err != nil { + errs = append(errs, fmt.Errorf("loading rules for file %s: %w", p, err)) + continue + } + res, err := r.RunFile(p, nil, ruleTypes) + if err != nil { + errs = append(errs, fmt.Errorf("error running file %s: %w", p, err)) + continue + } + allResults = append(allResults, res...) + } + } + return allResults, errors.Join(errs...) +} + +// TestDir discovers and executes all *.star test files under the given +// directory, reporting results through t. It also loads *.yaml rules. +func (r *Runner) TestDir(t *testing.T, dir string) { t.Helper() + ruleTypes, err := loadRulesFromDir(dir) + if err != nil { + t.Fatalf("loading rules: %v", err) + } + files, err := DiscoverFiles(dir) if err != nil { t.Fatalf("discovering test files: %v", err) @@ -196,7 +294,7 @@ func (r *Runner) RunDir(t *testing.T, dir string) { } t.Run(rel, func(t *testing.T) { - results, err := r.RunFile(file, nil) + results, err := r.RunFile(file, nil, ruleTypes) if err != nil { t.Fatalf("running %s: %v", file, err) } diff --git a/pkg/ruletest/runner_test.go b/pkg/ruletest/runner_test.go index 2471b5397a..f499b8c3d2 100644 --- a/pkg/ruletest/runner_test.go +++ b/pkg/ruletest/runner_test.go @@ -4,6 +4,7 @@ package ruletest import ( + "os" "path/filepath" "sort" "strings" @@ -37,7 +38,7 @@ func TestDiscoverFiles(t *testing.T) { func TestRunFile(t *testing.T) { t.Parallel() r := NewRunner() - results, err := r.RunFile(filepath.Join("testdata", "sample.star"), nil) + results, err := r.RunFile(filepath.Join("testdata", "sample.star"), nil, nil) if err != nil { t.Fatalf("RunFile failed: %v", err) } @@ -107,7 +108,7 @@ func TestRunEvalFile(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - results, err := r.RunFile(filepath.Join("testdata", tt.file), nil) + results, err := r.RunFile(filepath.Join("testdata", tt.file), nil, nil) if err != nil { t.Fatalf("RunFile failed for %s: %v", tt.file, err) } @@ -125,3 +126,17 @@ func TestRunEvalFile(t *testing.T) { }) } } + +func TestTestDir(t *testing.T) { + t.Parallel() + r := NewRunner() + dir := t.TempDir() + content := []byte(` +def test_pass(): + assert.eq(1, 1) +`) + if err := os.WriteFile(filepath.Join(dir, "pass.star"), content, 0644); err != nil { + t.Fatal(err) + } + r.TestDir(t, dir) +} From 0b195dbfbf2eeb8ffabb709ef1b5d35a8d9fb707 Mon Sep 17 00:00:00 2001 From: krrish175-byte Date: Tue, 30 Jun 2026 00:14:43 +0530 Subject: [PATCH 2/8] fix: address reviewer feedback on mindev test command and ruletest - Fix GCI import ordering in runner.go (minder imports in separate section) - Restore errors.AsType in RunFile for proper error-wrapping support - Make RunDir private; TestDir now delegates to RunPaths to remove duplication - Rename ruleNameOrPath to ruleName in builtinEval for clarity - Update mindev test Use field to accept files or directories --- cmd/dev/app/test/test.go | 5 +- pkg/ruletest/eval.go | 87 ++++++++++++++++++++---------- pkg/ruletest/eval_test.go | 23 ++++++++ pkg/ruletest/runner.go | 49 ++++++----------- pkg/testkit/v1/testkit.go | 10 ++++ pkg/testkit/v1/testkit_provider.go | 62 +++++++++++++++++++-- 6 files changed, 169 insertions(+), 67 deletions(-) diff --git a/cmd/dev/app/test/test.go b/cmd/dev/app/test/test.go index b09dfd5924..376999e033 100644 --- a/cmd/dev/app/test/test.go +++ b/cmd/dev/app/test/test.go @@ -15,9 +15,10 @@ import ( // CmdTest returns the test cobra command func CmdTest() *cobra.Command { cmd := &cobra.Command{ - Use: "test [directories...]", + Use: "test [paths...]", Short: "Run Minder rule tests", - Long: `Run Starlark-based tests for Minder rules. If no directories are provided, tests the current directory.`, + Long: "Run Starlark-based tests for Minder rules. Each path may be a file or directory. " + + "If no paths are provided, tests the current directory.", RunE: func(cmd *cobra.Command, args []string) error { if len(args) == 0 { args = []string{"."} diff --git a/pkg/ruletest/eval.go b/pkg/ruletest/eval.go index e402dc6e9b..f39bfd80cc 100644 --- a/pkg/ruletest/eval.go +++ b/pkg/ruletest/eval.go @@ -24,44 +24,26 @@ import ( func (tr *testCaseRunner) builtinEval( thread *starlark.Thread, _ *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple, ) (starlark.Value, error) { - var ruleNameOrPath string + var ruleName string var entityDict *starlark.Dict var profileDict *starlark.Dict var mockHttpDict *starlark.Dict + var mockFSDict *starlark.Dict err := starlark.UnpackArgs("eval", args, kwargs, - "rule", &ruleNameOrPath, "entity?", &entityDict, "profile?", &profileDict, "mock_http?", &mockHttpDict) + "rule", &ruleName, "entity?", &entityDict, "profile?", &profileDict, "mock_http?", &mockHttpDict, "mock_fs?", &mockFSDict) if err != nil { return nil, err } - var rt *minderv1.RuleType - - if tr.ruleTypes != nil { - if ruleType, ok := tr.ruleTypes[ruleNameOrPath]; ok { - rt = ruleType - } + mockFSMap, err := parseMockFSDict(mockFSDict) + if err != nil { + return nil, err } - if rt == nil { - rulePath := ruleNameOrPath - if !filepath.IsAbs(rulePath) { - callerFrame := thread.CallFrame(1) - if callerFile := callerFrame.Pos.Filename(); callerFile != "" { - rulePath = filepath.Join(filepath.Dir(callerFile), rulePath) - } - } - - decoder, closer := fileconvert.DecoderForFile(rulePath) - if decoder == nil { - return nil, fmt.Errorf("error opening file: %s (or rule not found in loaded rule types)", rulePath) - } - defer closer.Close() - - rt, err = fileconvert.ReadResourceTyped[*minderv1.RuleType](decoder) - if err != nil { - return nil, fmt.Errorf("failed to parse rule type: %w", err) - } + rt, err := tr.loadRuleTypeFallback(ruleName, thread) + if err != nil { + return nil, err } profileMap, err := dictToGoMap(profileDict) @@ -86,7 +68,11 @@ func (tr *testCaseRunner) builtinEval( ctx := context.Background() - tk := tkv1.NewTestKit(tkv1.WithHandlerFunc(mockHandler.ServeHTTP)) + tkOpts := []tkv1.Option{tkv1.WithHandlerFunc(mockHandler.ServeHTTP)} + if len(mockFSMap) > 0 { + tkOpts = append(tkOpts, tkv1.WithMockFS(mockFSMap)) + } + tk := tkv1.NewTestKit(tkOpts...) rte, err := rtengine.NewRuleTypeEngine(ctx, rt, tk) if err != nil { @@ -132,6 +118,51 @@ func formatEvalResult(evalErr error) *starlark.Dict { return result } +func parseMockFSDict(mockFSDict *starlark.Dict) (map[string]string, error) { + mockFSMap := make(map[string]string) + if mockFSDict != nil { + for _, item := range mockFSDict.Items() { + k, v := item[0], item[1] + ks, ok1 := k.(starlark.String) + vs, ok2 := v.(starlark.String) + if !ok1 || !ok2 { + return nil, fmt.Errorf("mock_fs keys and values must be strings") + } + mockFSMap[string(ks)] = string(vs) + } + } + return mockFSMap, nil +} + +func (tr *testCaseRunner) loadRuleTypeFallback(ruleName string, thread *starlark.Thread) (*minderv1.RuleType, error) { + if tr.ruleTypes != nil { + if ruleType, ok := tr.ruleTypes[ruleName]; ok { + return ruleType, nil + } + } + + rulePath := ruleName + if !filepath.IsAbs(rulePath) { + callerFrame := thread.CallFrame(1) + if callerFile := callerFrame.Pos.Filename(); callerFile != "" { + rulePath = filepath.Join(filepath.Dir(callerFile), rulePath) + } + } + + decoder, closer := fileconvert.DecoderForFile(rulePath) + if decoder == nil { + return nil, fmt.Errorf("rule %q not found in loaded rule types and no file found at path %s", ruleName, rulePath) + } + defer closer.Close() + + rt, err := fileconvert.ReadResourceTyped[*minderv1.RuleType](decoder) + if err != nil { + return nil, fmt.Errorf("failed to parse rule type: %w", err) + } + + return rt, nil +} + //nolint:gocyclo // this is a simple switch over many entity types func mapToProto(entityType string, entityMap map[string]any) (proto.Message, error) { if len(entityMap) == 0 { diff --git a/pkg/ruletest/eval_test.go b/pkg/ruletest/eval_test.go index f1798f1b72..cbfbd9fbfb 100644 --- a/pkg/ruletest/eval_test.go +++ b/pkg/ruletest/eval_test.go @@ -95,6 +95,29 @@ func TestBuiltinEval_InvalidArgs(t *testing.T) { kwargs: []starlark.Tuple{{starlark.String("entity"), starlark.String("not a dict")}}, wantErr: "got string, want dict", }, + { + name: "invalid mock_fs type", + args: starlark.Tuple{starlark.String("rule.yaml")}, + kwargs: []starlark.Tuple{ + {starlark.String("mock_fs"), starlark.String("not a dict")}, + }, + wantErr: "got string, want dict", + }, + { + name: "mock_fs non-string keys", + args: starlark.Tuple{starlark.String("rule.yaml")}, + kwargs: []starlark.Tuple{ + { + starlark.String("mock_fs"), + func() *starlark.Dict { + d := starlark.NewDict(1) + _ = d.SetKey(starlark.MakeInt(1), starlark.String("content")) + return d + }(), + }, + }, + wantErr: "mock_fs keys and values must be strings", + }, } for _, tt := range tests { diff --git a/pkg/ruletest/runner.go b/pkg/ruletest/runner.go index e4900ada3e..b75d85aa2f 100644 --- a/pkg/ruletest/runner.go +++ b/pkg/ruletest/runner.go @@ -15,11 +15,12 @@ import ( "strings" "testing" - minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" - "github.com/mindersec/minder/pkg/fileconvert" "go.starlark.net/starlark" "go.starlark.net/starlarktest" "go.starlark.net/syntax" + + minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" + "github.com/mindersec/minder/pkg/fileconvert" ) // testCaseRunner is responsible for executing a single Starlark file @@ -110,7 +111,7 @@ func (r *Runner) RunFile(filename string, src any, ruleTypes map[string]*minderv globals, err := tr.runFile(filename, src) if err != nil { - if evalErr, ok := err.(*starlark.EvalError); ok { + if evalErr, ok := errors.AsType[*starlark.EvalError](err); ok { return nil, fmt.Errorf("loading %s: %w\n%s", filename, err, evalErr.Backtrace()) } return nil, fmt.Errorf("loading %s: %w", filename, err) @@ -207,9 +208,9 @@ func loadRulesFromDir(dir string) (map[string]*minderv1.RuleType, error) { return ruleTypes, nil } -// RunDir discovers and executes all *.star test files under the given +// runDir discovers and executes all *.star test files under the given // directory. It also discovers and loads any *.yaml rule files in the directory. -func (r *Runner) RunDir(dir string) ([]TestResult, error) { +func (r *Runner) runDir(dir string) ([]TestResult, error) { ruleTypes, err := loadRulesFromDir(dir) if err != nil { return nil, fmt.Errorf("loading rules: %w", err) @@ -245,7 +246,7 @@ func (r *Runner) RunPaths(paths []string) ([]TestResult, error) { continue } if info.IsDir() { - res, err := r.RunDir(p) + res, err := r.runDir(p) if err != nil { errs = append(errs, fmt.Errorf("error running directory %s: %w", p, err)) } @@ -271,40 +272,24 @@ func (r *Runner) RunPaths(paths []string) ([]TestResult, error) { // directory, reporting results through t. It also loads *.yaml rules. func (r *Runner) TestDir(t *testing.T, dir string) { t.Helper() + t.Parallel() - ruleTypes, err := loadRulesFromDir(dir) - if err != nil { - t.Fatalf("loading rules: %v", err) - } - - files, err := DiscoverFiles(dir) + results, err := r.RunPaths([]string{dir}) if err != nil { - t.Fatalf("discovering test files: %v", err) + t.Fatalf("running tests in %s: %v", dir, err) } - if len(files) == 0 { + if len(results) == 0 { t.Logf("no *.star test files found in %s", dir) return } - for _, file := range files { - rel, err := filepath.Rel(dir, file) - if err != nil { - t.Fatalf("failed to compute relative path for %s: %v", file, err) - } - - t.Run(rel, func(t *testing.T) { - results, err := r.RunFile(file, nil, ruleTypes) - if err != nil { - t.Fatalf("running %s: %v", file, err) - } - - for _, result := range results { - t.Run(result.Name, func(t *testing.T) { - for _, msg := range result.Failures { - t.Error(msg) - } - }) + for _, result := range results { + result := result + t.Run(result.Name, func(t *testing.T) { + t.Parallel() + for _, msg := range result.Failures { + t.Error(msg) } }) } diff --git a/pkg/testkit/v1/testkit.go b/pkg/testkit/v1/testkit.go index 67d8a715ab..c943a2520b 100644 --- a/pkg/testkit/v1/testkit.go +++ b/pkg/testkit/v1/testkit.go @@ -18,6 +18,9 @@ type TestKit struct { // gitDir is the directory where the git repository is cloned gitDir string + // mockFS contains the filesystem representation for git ingestion testing + mockFS map[string]string + // HTTP httpHandler http.Handler } @@ -35,6 +38,13 @@ func WithGitDir(dir string) Option { } } +// WithMockFS is a functional option to configure the TestKit with a mocked filesystem +func WithMockFS(fs map[string]string) Option { + return func(tk *TestKit) { + tk.mockFS = fs + } +} + // WithHandlerFunc is a functional option to configure the TestKit to use a specific Handler func WithHandlerFunc(hf http.HandlerFunc) Option { return func(tk *TestKit) { diff --git a/pkg/testkit/v1/testkit_provider.go b/pkg/testkit/v1/testkit_provider.go index 53437cc64a..9b57fd368d 100644 --- a/pkg/testkit/v1/testkit_provider.go +++ b/pkg/testkit/v1/testkit_provider.go @@ -6,8 +6,13 @@ package v1 import ( "context" "errors" + "path/filepath" + "time" + "github.com/go-git/go-billy/v5/memfs" "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/plumbing/object" + "github.com/go-git/go-git/v5/storage/memory" "google.golang.org/protobuf/reflect/protoreflect" minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" @@ -83,9 +88,56 @@ func (*TestKit) PropertiesToProtoMessage(_ minderv1.Entity, _ *properties.Proper return nil, nil } -// Clone Implements the Git trait. This is a stub implementation that allows us to instantiate a Git ingester. -// This will later be overridden by the actual implementation. -func (*TestKit) Clone(_ context.Context, _ string, _ string) (*git.Repository, error) { - // Note that this should not be called. If it is, it means that the ingester has not been overridden. - return nil, ErrNotIngesterOverridden +// Clone Implements the Git trait. This initializes an in-memory repository with the mocked filesystem if provided. +func (tk *TestKit) Clone(_ context.Context, _ string, _ string) (*git.Repository, error) { + if len(tk.mockFS) == 0 { + return nil, ErrNotIngesterOverridden + } + + storer := memory.NewStorage() + fs := memfs.New() + + for path, content := range tk.mockFS { + if err := fs.MkdirAll(filepath.Dir(path), 0755); err != nil { + return nil, err + } + f, err := fs.Create(path) + if err != nil { + return nil, err + } + if _, err := f.Write([]byte(content)); err != nil { + _ = f.Close() + return nil, err + } + _ = f.Close() + } + + repo, err := git.Init(storer, fs) + if err != nil { + return nil, err + } + + w, err := repo.Worktree() + if err != nil { + return nil, err + } + + for path := range tk.mockFS { + if _, err := w.Add(path); err != nil { + return nil, err + } + } + + _, err = w.Commit("Initial commit", &git.CommitOptions{ + Author: &object.Signature{ + Name: "TestKit", + Email: "testkit@minder.test", + When: time.Now(), + }, + }) + if err != nil { + return nil, err + } + + return repo, nil } From 96953ef0b97ac61ff75be9ce58623ec4c6b46a82 Mon Sep 17 00:00:00 2001 From: krrish175-byte Date: Wed, 1 Jul 2026 12:03:20 +0530 Subject: [PATCH 3/8] fix: address second round of reviewer feedback on ruletest - Remove t.Parallel() from TestDir to prevent panic when caller already called it - Add Filename field to TestResult; TestDir groups sub-tests by file/name - Restore more descriptive RunFile doc comment explaining src parameter - Remove path fallback from rule lookup: eval() now only accepts rule names, not file paths, resolving the dual-typed argument complexity - Update testdata/eval.star to reference rule by name (branch_protection_reviews) instead of file path (rule_type_sample.yaml) - Load ruleTypes in TestRunEvalFile so name-based lookup works in tests - Remove nil guard from mapToProto: empty entity maps now correctly flow through to the engine instead of silently returning nil --- pkg/ruletest/eval.go | 35 +++------------------------------ pkg/ruletest/runner.go | 13 +++++++----- pkg/ruletest/runner_test.go | 7 ++++++- pkg/ruletest/testdata/eval.star | 6 +++--- 4 files changed, 20 insertions(+), 41 deletions(-) diff --git a/pkg/ruletest/eval.go b/pkg/ruletest/eval.go index f39bfd80cc..9816606b59 100644 --- a/pkg/ruletest/eval.go +++ b/pkg/ruletest/eval.go @@ -8,7 +8,6 @@ import ( "encoding/json" "errors" "fmt" - "path/filepath" "go.starlark.net/starlark" "google.golang.org/protobuf/encoding/protojson" @@ -17,7 +16,6 @@ import ( minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" "github.com/mindersec/minder/pkg/engine/v1/interfaces" "github.com/mindersec/minder/pkg/engine/v1/rtengine" - "github.com/mindersec/minder/pkg/fileconvert" tkv1 "github.com/mindersec/minder/pkg/testkit/v1" ) @@ -41,7 +39,7 @@ func (tr *testCaseRunner) builtinEval( return nil, err } - rt, err := tr.loadRuleTypeFallback(ruleName, thread) + rt, err := tr.lookupRuleType(ruleName) if err != nil { return nil, err } @@ -134,41 +132,17 @@ func parseMockFSDict(mockFSDict *starlark.Dict) (map[string]string, error) { return mockFSMap, nil } -func (tr *testCaseRunner) loadRuleTypeFallback(ruleName string, thread *starlark.Thread) (*minderv1.RuleType, error) { +func (tr *testCaseRunner) lookupRuleType(ruleName string) (*minderv1.RuleType, error) { if tr.ruleTypes != nil { if ruleType, ok := tr.ruleTypes[ruleName]; ok { return ruleType, nil } } - - rulePath := ruleName - if !filepath.IsAbs(rulePath) { - callerFrame := thread.CallFrame(1) - if callerFile := callerFrame.Pos.Filename(); callerFile != "" { - rulePath = filepath.Join(filepath.Dir(callerFile), rulePath) - } - } - - decoder, closer := fileconvert.DecoderForFile(rulePath) - if decoder == nil { - return nil, fmt.Errorf("rule %q not found in loaded rule types and no file found at path %s", ruleName, rulePath) - } - defer closer.Close() - - rt, err := fileconvert.ReadResourceTyped[*minderv1.RuleType](decoder) - if err != nil { - return nil, fmt.Errorf("failed to parse rule type: %w", err) - } - - return rt, nil + return nil, fmt.Errorf("rule %q not found; make sure the rule type YAML is in the same directory as the test file", ruleName) } //nolint:gocyclo // this is a simple switch over many entity types func mapToProto(entityType string, entityMap map[string]any) (proto.Message, error) { - if len(entityMap) == 0 { - return nil, nil - } - b, err := json.Marshal(entityMap) if err != nil { return nil, err @@ -197,9 +171,6 @@ func mapToProto(entityType string, entityMap map[string]any) (proto.Message, err minderv1.Entity_ENTITY_PULL_REQUESTS: fallthrough default: - // Some entities like PullRequest or BuildEnvironment may not have concrete protobuf structs available here. - // For mocking purposes, returning nil is acceptable if the template doesn't strict check them, - // but returning an error is safer to flag unsupported mocking right now. return nil, fmt.Errorf("unsupported entity type for mapping to proto: %s", entityType) } diff --git a/pkg/ruletest/runner.go b/pkg/ruletest/runner.go index b75d85aa2f..3e1ba79756 100644 --- a/pkg/ruletest/runner.go +++ b/pkg/ruletest/runner.go @@ -71,6 +71,7 @@ func (tr *testCaseRunner) Error(args ...any) { // TestResult holds the outcome of a single Starlark test function. type TestResult struct { + Filename string Name string Failures []string } @@ -96,8 +97,9 @@ func NewRunner() *Runner { } } -// RunFile executes a single Starlark test file. If src is non-nil, it is -// used as the file contents. +// RunFile executes a single Starlark test file and returns the results +// for each test_* function found in it. +// src may be nil, or a string, []byte, or io.Reader containing the file source. func (r *Runner) RunFile(filename string, src any, ruleTypes map[string]*minderv1.RuleType) ([]TestResult, error) { if filename == "" { return nil, errors.New("filename cannot be empty") @@ -132,9 +134,11 @@ func (r *Runner) RunFile(filename string, src any, ruleTypes map[string]*minderv testFns[name] = fn } + base := filepath.Base(filename) var results []TestResult for name, fn := range testFns { result := r.runOneTest(name, fn, fileSystem, ruleTypes) + result.Filename = base results = append(results, result) } @@ -272,7 +276,6 @@ func (r *Runner) RunPaths(paths []string) ([]TestResult, error) { // directory, reporting results through t. It also loads *.yaml rules. func (r *Runner) TestDir(t *testing.T, dir string) { t.Helper() - t.Parallel() results, err := r.RunPaths([]string{dir}) if err != nil { @@ -286,8 +289,8 @@ func (r *Runner) TestDir(t *testing.T, dir string) { for _, result := range results { result := result - t.Run(result.Name, func(t *testing.T) { - t.Parallel() + name := result.Filename + "/" + result.Name + t.Run(name, func(t *testing.T) { for _, msg := range result.Failures { t.Error(msg) } diff --git a/pkg/ruletest/runner_test.go b/pkg/ruletest/runner_test.go index f499b8c3d2..2a86e1b22f 100644 --- a/pkg/ruletest/runner_test.go +++ b/pkg/ruletest/runner_test.go @@ -104,11 +104,16 @@ func TestRunEvalFile(t *testing.T) { {name: "builtins", file: "builtins_test.star"}, } + ruleTypes, err := loadRulesFromDir("testdata") + if err != nil { + t.Fatalf("loading rule types: %v", err) + } + r := NewRunner() for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - results, err := r.RunFile(filepath.Join("testdata", tt.file), nil, nil) + results, err := r.RunFile(filepath.Join("testdata", tt.file), nil, ruleTypes) if err != nil { t.Fatalf("RunFile failed for %s: %v", tt.file, err) } diff --git a/pkg/ruletest/testdata/eval.star b/pkg/ruletest/testdata/eval.star index 886e777ea3..bf8435fe37 100644 --- a/pkg/ruletest/testdata/eval.star +++ b/pkg/ruletest/testdata/eval.star @@ -3,7 +3,7 @@ def test_eval_success(): res = eval( - rule="rule_type_sample.yaml", + rule="branch_protection_reviews", entity={"owner": "test", "name": "repo"}, profile={"required_reviews": 2}, mock_http={ @@ -14,7 +14,7 @@ def test_eval_success(): def test_eval_fail(): res = eval( - rule="rule_type_sample.yaml", + rule="branch_protection_reviews", entity={"owner": "test", "name": "repo"}, profile={"required_reviews": 2}, mock_http={ @@ -26,7 +26,7 @@ def test_eval_fail(): def test_eval_error_404(): res = eval( - rule="rule_type_sample.yaml", + rule="branch_protection_reviews", entity={"owner": "test", "name": "repo"}, profile={"required_reviews": 2}, mock_http={ From faee005588f0f9ca1df710d8a3aef554ba9bde9a Mon Sep 17 00:00:00 2001 From: krrish175-byte Date: Thu, 2 Jul 2026 13:45:46 +0530 Subject: [PATCH 4/8] fix: address PR #6551 review comments - Addressed Evan's feedback on testkit mocking (refactored to gitFS/WithGitFiles) - Fixed 'err' return values in cmd/dev/app/test/test.go - Added filename prefix to CLI test output - Fixed defer Close() pattern in ruletest/runner.go - Updated RunFile godoc --- cmd/dev/app/test/test.go | 12 +-- pkg/ruletest/eval.go | 2 +- pkg/ruletest/runner.go | 121 +++++++++++------------- pkg/ruletest/runner_test.go | 25 +---- pkg/ruletest/testdata/mock_fs.star | 24 +++++ pkg/ruletest/testdata/mock_fs_rule.yaml | 30 ++++++ pkg/testkit/v1/testkit.go | 49 ++++++---- pkg/testkit/v1/testkit_ingest.go | 9 +- pkg/testkit/v1/testkit_provider.go | 34 +++---- 9 files changed, 161 insertions(+), 145 deletions(-) create mode 100644 pkg/ruletest/testdata/mock_fs.star create mode 100644 pkg/ruletest/testdata/mock_fs_rule.yaml diff --git a/cmd/dev/app/test/test.go b/cmd/dev/app/test/test.go index 376999e033..a579930d3e 100644 --- a/cmd/dev/app/test/test.go +++ b/cmd/dev/app/test/test.go @@ -5,7 +5,7 @@ package test import ( - "fmt" + "errors" "github.com/spf13/cobra" @@ -32,27 +32,27 @@ func CmdTest() *cobra.Command { if len(results) == 0 { cmd.Printf("No tests found\n") - return err + return nil } hasFailures := false for _, res := range results { if len(res.Failures) > 0 { hasFailures = true - cmd.Printf("FAIL: %s\n", res.Name) + cmd.Printf("FAIL: %s/%s\n", res.Filename, res.Name) for _, f := range res.Failures { cmd.Printf(" - %s\n", f) } } else { - cmd.Printf("PASS: %s\n", res.Name) + cmd.Printf("PASS: %s/%s\n", res.Filename, res.Name) } } if hasFailures { - return fmt.Errorf("one or more tests failed") + return errors.New("one or more tests failed") } - return err + return nil }, } diff --git a/pkg/ruletest/eval.go b/pkg/ruletest/eval.go index 9816606b59..d60226e5a9 100644 --- a/pkg/ruletest/eval.go +++ b/pkg/ruletest/eval.go @@ -68,7 +68,7 @@ func (tr *testCaseRunner) builtinEval( tkOpts := []tkv1.Option{tkv1.WithHandlerFunc(mockHandler.ServeHTTP)} if len(mockFSMap) > 0 { - tkOpts = append(tkOpts, tkv1.WithMockFS(mockFSMap)) + tkOpts = append(tkOpts, tkv1.WithGitFiles(mockFSMap)) } tk := tkv1.NewTestKit(tkOpts...) diff --git a/pkg/ruletest/runner.go b/pkg/ruletest/runner.go index 3e1ba79756..791e821eac 100644 --- a/pkg/ruletest/runner.go +++ b/pkg/ruletest/runner.go @@ -19,6 +19,7 @@ import ( "go.starlark.net/starlarktest" "go.starlark.net/syntax" + "github.com/mindersec/minder/internal/util" minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" "github.com/mindersec/minder/pkg/fileconvert" ) @@ -99,7 +100,11 @@ func NewRunner() *Runner { // RunFile executes a single Starlark test file and returns the results // for each test_* function found in it. -// src may be nil, or a string, []byte, or io.Reader containing the file source. +// +// filename is the path to the *.star file. If src is nil, the file is +// read from disk; otherwise src may be a string, []byte, or io.Reader +// containing the Starlark source. ruleTypes supplies the rule type +// definitions available to eval() calls within the test file. func (r *Runner) RunFile(filename string, src any, ruleTypes map[string]*minderv1.RuleType) ([]TestResult, error) { if filename == "" { return nil, errors.New("filename cannot be empty") @@ -168,26 +173,6 @@ func (r *Runner) runOneTest( return result } -// DiscoverFiles walks the given directory tree and returns the paths of -// all *.star files found, in sorted order. -func DiscoverFiles(root string) ([]string, error) { - var files []string - err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { - if err != nil { - return err - } - if !d.IsDir() && strings.HasSuffix(d.Name(), ".star") { - files = append(files, path) - } - return nil - }) - if err != nil { - return nil, fmt.Errorf("walking %s: %w", root, err) - } - sort.Strings(files) - return files, nil -} - // loadRulesFromDir finds and parses all *.yaml files in the given directory // into a map of RuleTypes keyed by rule name. func loadRulesFromDir(dir string) (map[string]*minderv1.RuleType, error) { @@ -197,73 +182,76 @@ func loadRulesFromDir(dir string) (map[string]*minderv1.RuleType, error) { return nil, fmt.Errorf("globbing yaml files: %w", err) } for _, yf := range yamlFiles { - decoder, closer := fileconvert.DecoderForFile(yf) - if decoder == nil { - return nil, fmt.Errorf("error opening file: %s", yf) + rt, err := loadSingleRule(yf) + if err != nil { + continue // skip files that aren't valid rule types } - defer func(c io.Closer) { - _ = c.Close() - }(closer) - rt, err := fileconvert.ReadResourceTyped[*minderv1.RuleType](decoder) - if err == nil && rt != nil && rt.Name != "" { + if rt != nil && rt.Name != "" { ruleTypes[rt.Name] = rt } } return ruleTypes, nil } -// runDir discovers and executes all *.star test files under the given -// directory. It also discovers and loads any *.yaml rule files in the directory. -func (r *Runner) runDir(dir string) ([]TestResult, error) { - ruleTypes, err := loadRulesFromDir(dir) - if err != nil { - return nil, fmt.Errorf("loading rules: %w", err) +// loadSingleRule reads a single YAML file and returns the parsed RuleType, if any. +func loadSingleRule(path string) (*minderv1.RuleType, error) { + decoder, closer := fileconvert.DecoderForFile(path) + if decoder == nil { + return nil, fmt.Errorf("error opening file: %s", path) } + defer func(c io.Closer) { + _ = c.Close() + }(closer) + return fileconvert.ReadResourceTyped[*minderv1.RuleType](decoder) +} - files, err := DiscoverFiles(dir) +// RunPaths takes a list of file or directory paths, discovering all *.star test +// files recursively. Tests are grouped by their immediate directory, and any +// *.yaml rules in that same directory are loaded and made available to the tests. +// It collects errors instead of returning early on the first error. +func (r *Runner) RunPaths(paths []string) ([]TestResult, error) { + expanded, err := util.ExpandFileArgs(paths...) if err != nil { - return nil, fmt.Errorf("discovering test files: %w", err) + return nil, fmt.Errorf("expanding paths: %w", err) } - var allResults []TestResult - for _, file := range files { - results, err := r.RunFile(file, nil, ruleTypes) - if err != nil { - return nil, err + // Group .star files by their immediate directory + filesByDir := make(map[string][]string) + for _, f := range expanded { + if !f.Expanded && filepath.Ext(f.Path) != ".star" && filepath.Ext(f.Path) != ".yaml" && filepath.Ext(f.Path) != ".yml" { + // If it's a specific file that is not a star or yaml file, we skip it + continue + } + if filepath.Ext(f.Path) == ".star" { + dir := filepath.Dir(f.Path) + filesByDir[dir] = append(filesByDir[dir], f.Path) } - allResults = append(allResults, results...) } - return allResults, nil -} - -// RunPaths takes a list of file or directory paths, executing tests in each. -// It collects errors instead of returning early on the first error. -func (r *Runner) RunPaths(paths []string) ([]TestResult, error) { var allResults []TestResult var errs []error - for _, p := range paths { - info, err := os.Stat(p) + // Ensure deterministic execution order by sorting directories + var dirs []string + for dir := range filesByDir { + dirs = append(dirs, dir) + } + sort.Strings(dirs) + + for _, dir := range dirs { + files := filesByDir[dir] + sort.Strings(files) + + ruleTypes, err := loadRulesFromDir(dir) if err != nil { - errs = append(errs, fmt.Errorf("stat %s: %w", p, err)) + errs = append(errs, fmt.Errorf("loading rules for directory %s: %w", dir, err)) continue } - if info.IsDir() { - res, err := r.runDir(p) - if err != nil { - errs = append(errs, fmt.Errorf("error running directory %s: %w", p, err)) - } - allResults = append(allResults, res...) - } else { - ruleTypes, err := loadRulesFromDir(filepath.Dir(p)) - if err != nil { - errs = append(errs, fmt.Errorf("loading rules for file %s: %w", p, err)) - continue - } - res, err := r.RunFile(p, nil, ruleTypes) + + for _, file := range files { + res, err := r.RunFile(file, nil, ruleTypes) if err != nil { - errs = append(errs, fmt.Errorf("error running file %s: %w", p, err)) + errs = append(errs, fmt.Errorf("error running file %s: %w", file, err)) continue } allResults = append(allResults, res...) @@ -274,6 +262,7 @@ func (r *Runner) RunPaths(paths []string) ([]TestResult, error) { // TestDir discovers and executes all *.star test files under the given // directory, reporting results through t. It also loads *.yaml rules. +//nolint:tparallel // TestDir acts as a test runner helper, caller is responsible for t.Parallel func (r *Runner) TestDir(t *testing.T, dir string) { t.Helper() diff --git a/pkg/ruletest/runner_test.go b/pkg/ruletest/runner_test.go index 2a86e1b22f..d584c189a6 100644 --- a/pkg/ruletest/runner_test.go +++ b/pkg/ruletest/runner_test.go @@ -11,30 +11,6 @@ import ( "testing" ) -func TestDiscoverFiles(t *testing.T) { - t.Parallel() - files, err := DiscoverFiles("testdata") - if err != nil { - t.Fatalf("DiscoverFiles failed: %v", err) - } - - found := make(map[string]bool) - for _, f := range files { - found[f] = true - } - - expected := []string{ - filepath.Join("testdata", "eval.star"), - filepath.Join("testdata", "sample.star"), - } - - for _, exp := range expected { - if !found[exp] { - t.Errorf("expected to find %s, but it was missing", exp) - } - } -} - func TestRunFile(t *testing.T) { t.Parallel() r := NewRunner() @@ -102,6 +78,7 @@ func TestRunEvalFile(t *testing.T) { }{ {name: "eval", file: "eval.star"}, {name: "builtins", file: "builtins_test.star"}, + {name: "mock_fs", file: "mock_fs.star"}, } ruleTypes, err := loadRulesFromDir("testdata") diff --git a/pkg/ruletest/testdata/mock_fs.star b/pkg/ruletest/testdata/mock_fs.star new file mode 100644 index 0000000000..7e53d26f7a --- /dev/null +++ b/pkg/ruletest/testdata/mock_fs.star @@ -0,0 +1,24 @@ +# SPDX-FileCopyrightText: Copyright 2024 The Minder Authors +# SPDX-License-Identifier: Apache-2.0 + +def test_mock_fs_success(): + res = eval( + rule="mock_fs_rule", + entity={"owner": "test", "name": "repo", "clone_url": "https://github.com/test/repo.git"}, + mock_fs={ + "test.txt": "hello world\n" + } + ) + assert.eq(res["message"], "") + assert.eq(res["status"], "pass") + +def test_mock_fs_fail(): + res = eval( + rule="mock_fs_rule", + entity={"owner": "test", "name": "repo", "clone_url": "https://github.com/test/repo.git"}, + mock_fs={ + "test.txt": "goodbye world\n" + } + ) + assert.true(res["message"] != "") + assert.eq(res["status"], "fail") diff --git a/pkg/ruletest/testdata/mock_fs_rule.yaml b/pkg/ruletest/testdata/mock_fs_rule.yaml new file mode 100644 index 0000000000..813f81cb97 --- /dev/null +++ b/pkg/ruletest/testdata/mock_fs_rule.yaml @@ -0,0 +1,30 @@ +type: rule-type +version: v1 +name: mock_fs_rule +display_name: Mock FS Rule +release_phase: alpha +context: + provider: github + project: 00000000-0000-0000-0000-000000000000 +severity: + value: info +def: + in_entity: repository + ingest: + type: git + rule_schema: {} + eval: + type: rego + rego: + type: deny-by-default + def: | + package minder + + import rego.v1 + + default allow := false + + allow if { + file.exists("test.txt") + file.read("test.txt") == "hello world\n" + } diff --git a/pkg/testkit/v1/testkit.go b/pkg/testkit/v1/testkit.go index c943a2520b..7ec6e95a37 100644 --- a/pkg/testkit/v1/testkit.go +++ b/pkg/testkit/v1/testkit.go @@ -7,19 +7,19 @@ package v1 import ( "net/http" + "path/filepath" - "github.com/mindersec/minder/internal/engine/ingester/git" + "github.com/go-git/go-billy/v5" + "github.com/go-git/go-billy/v5/memfs" + "github.com/go-git/go-billy/v5/osfs" ) // TestKit implements a set of interfaces for testing // purposes. e.g. for testing rule types. type TestKit struct { - ingestType string - // gitDir is the directory where the git repository is cloned - gitDir string - - // mockFS contains the filesystem representation for git ingestion testing - mockFS map[string]string + // gitFS is the filesystem used for git ingestion testing. + // Both WithGitDir and WithGitFiles populate this field. + gitFS billy.Filesystem // HTTP httpHandler http.Handler @@ -28,20 +28,33 @@ type TestKit struct { // Option is a functional option type for TestKit type Option func(*TestKit) -// WithGitDir is a functional option to set the git directory -// Note that if the `git` ingest type is used, you need to overwrite the -// ingester in the rule type engine. +// WithGitDir is a functional option to set the git directory. +// It eagerly creates an osfs.Filesystem rooted at dir. func WithGitDir(dir string) Option { - return func(tp *TestKit) { - tp.ingestType = git.GitRuleDataIngestType - tp.gitDir = dir + return func(tk *TestKit) { + tk.gitFS = osfs.New(dir) } } -// WithMockFS is a functional option to configure the TestKit with a mocked filesystem -func WithMockFS(fs map[string]string) Option { +// WithGitFiles is a functional option to configure the TestKit with a +// mocked in-memory filesystem for git ingestion testing. Each key is +// a file path and each value is the file content. +func WithGitFiles(files map[string]string) Option { return func(tk *TestKit) { - tk.mockFS = fs + fs := memfs.New() + for path, content := range files { + dir := filepath.Dir(path) + if dir != "" && dir != "." { + _ = fs.MkdirAll(dir, 0755) + } + f, err := fs.Create(path) + if err != nil { + continue + } + _, _ = f.Write([]byte(content)) + _ = f.Close() + } + tk.gitFS = fs } } @@ -65,9 +78,7 @@ func WithHTTP(status int, body []byte, headers map[string]string) Option { // NewTestKit creates a new TestKit func NewTestKit(opts ...Option) *TestKit { - pt := &TestKit{ - gitDir: ".", - } + pt := &TestKit{} for _, opt := range opts { opt(pt) diff --git a/pkg/testkit/v1/testkit_ingest.go b/pkg/testkit/v1/testkit_ingest.go index f442f51051..fb0091db51 100644 --- a/pkg/testkit/v1/testkit_ingest.go +++ b/pkg/testkit/v1/testkit_ingest.go @@ -7,10 +7,8 @@ import ( "context" "errors" - "github.com/go-git/go-billy/v5/osfs" "google.golang.org/protobuf/reflect/protoreflect" - "github.com/mindersec/minder/internal/engine/ingester/git" "github.com/mindersec/minder/pkg/engine/v1/interfaces" ) @@ -26,7 +24,7 @@ var _ interfaces.Ingester = &TestKit{} func (tk *TestKit) Ingest( ctx context.Context, ent protoreflect.ProtoMessage, params map[string]any, ) (*interfaces.Ingested, error) { - if tk.ingestType == git.GitRuleDataIngestType { + if tk.gitFS != nil { return tk.fakeGit(ctx, ent, params) } return nil, ErrIngestUnimplemented @@ -34,7 +32,7 @@ func (tk *TestKit) Ingest( // ShouldOverrideIngest returns true if the ingester should override the ingest func (tk *TestKit) ShouldOverrideIngest() bool { - return tk.ingestType == git.GitRuleDataIngestType + return tk.gitFS != nil } // GetType returns the type of the ingester @@ -50,8 +48,7 @@ func (*TestKit) GetConfig() protoreflect.ProtoMessage { func (tk *TestKit) fakeGit( _ context.Context, _ protoreflect.ProtoMessage, _ map[string]any, ) (*interfaces.Ingested, error) { - fs := osfs.New(tk.gitDir) return &interfaces.Ingested{ - Fs: fs, + Fs: tk.gitFS, }, nil } diff --git a/pkg/testkit/v1/testkit_provider.go b/pkg/testkit/v1/testkit_provider.go index 9b57fd368d..5aef0de3e6 100644 --- a/pkg/testkit/v1/testkit_provider.go +++ b/pkg/testkit/v1/testkit_provider.go @@ -6,10 +6,8 @@ package v1 import ( "context" "errors" - "path/filepath" "time" - "github.com/go-git/go-billy/v5/memfs" "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing/object" "github.com/go-git/go-git/v5/storage/memory" @@ -88,31 +86,16 @@ func (*TestKit) PropertiesToProtoMessage(_ minderv1.Entity, _ *properties.Proper return nil, nil } -// Clone Implements the Git trait. This initializes an in-memory repository with the mocked filesystem if provided. +// Clone Implements the Git trait. If gitFS is set, it initializes an +// in-memory Git repository backed by that filesystem. func (tk *TestKit) Clone(_ context.Context, _ string, _ string) (*git.Repository, error) { - if len(tk.mockFS) == 0 { + if tk.gitFS == nil { return nil, ErrNotIngesterOverridden } storer := memory.NewStorage() - fs := memfs.New() - for path, content := range tk.mockFS { - if err := fs.MkdirAll(filepath.Dir(path), 0755); err != nil { - return nil, err - } - f, err := fs.Create(path) - if err != nil { - return nil, err - } - if _, err := f.Write([]byte(content)); err != nil { - _ = f.Close() - return nil, err - } - _ = f.Close() - } - - repo, err := git.Init(storer, fs) + repo, err := git.Init(storer, tk.gitFS) if err != nil { return nil, err } @@ -122,9 +105,14 @@ func (tk *TestKit) Clone(_ context.Context, _ string, _ string) (*git.Repository return nil, err } - for path := range tk.mockFS { + // Add all files from the worktree + status, err := w.Status() + if err != nil { + return nil, err + } + for path := range status { if _, err := w.Add(path); err != nil { - return nil, err + continue } } From 873de46acb55d1f072d6dbd47a563d04bdd93fb1 Mon Sep 17 00:00:00 2001 From: krrish175-byte Date: Thu, 2 Jul 2026 18:49:28 +0530 Subject: [PATCH 5/8] Address remaining PR comments from Evan - Add test_fail_ prefix support to TestDir - Simplify runner_test.go to use TestDir - Rename intentionally failing tests in sample.star to use test_fail_ prefix --- pkg/ruletest/runner.go | 6 ++ pkg/ruletest/runner_test.go | 112 +----------------------------- pkg/ruletest/testdata/sample.star | 4 +- 3 files changed, 9 insertions(+), 113 deletions(-) diff --git a/pkg/ruletest/runner.go b/pkg/ruletest/runner.go index 791e821eac..bb315ba5eb 100644 --- a/pkg/ruletest/runner.go +++ b/pkg/ruletest/runner.go @@ -280,6 +280,12 @@ func (r *Runner) TestDir(t *testing.T, dir string) { result := result name := result.Filename + "/" + result.Name t.Run(name, func(t *testing.T) { + if strings.HasPrefix(result.Name, "test_fail_") { + if result.Passed() { + t.Errorf("expected test %s to fail, but it passed", result.Name) + } + return + } for _, msg := range result.Failures { t.Error(msg) } diff --git a/pkg/ruletest/runner_test.go b/pkg/ruletest/runner_test.go index d584c189a6..cafc6214e4 100644 --- a/pkg/ruletest/runner_test.go +++ b/pkg/ruletest/runner_test.go @@ -4,121 +4,11 @@ package ruletest import ( - "os" - "path/filepath" - "sort" - "strings" "testing" ) -func TestRunFile(t *testing.T) { - t.Parallel() - r := NewRunner() - results, err := r.RunFile(filepath.Join("testdata", "sample.star"), nil, nil) - if err != nil { - t.Fatalf("RunFile failed: %v", err) - } - - if len(results) != 3 { - t.Fatalf("expected 3 test functions to be discovered and run, got %d", len(results)) - } - - sort.Slice(results, func(i, j int) bool { - return results[i].Name < results[j].Name - }) - - tc := []struct { - name string - wantPassed bool - failures []string - }{ - { - name: "test_exception", - wantPassed: false, - }, - { - name: "test_failing", - wantPassed: false, - failures: []string{"this test failed intentionally"}, - }, - { - name: "test_passing", - wantPassed: true, - }, - } - - for i, tt := range tc { - if results[i].Name != tt.name { - t.Errorf("results[%d]: expected name %s, got %s", i, tt.name, results[i].Name) - } - if results[i].Passed() != tt.wantPassed { - t.Errorf("results[%d] (%s): expected passed=%v, got passed=%v", i, tt.name, tt.wantPassed, results[i].Passed()) - } - if !tt.wantPassed && len(results[i].Failures) == 0 { - t.Errorf("results[%d] (%s): expected failures but got none", i, tt.name) - } - for j, want := range tt.failures { - if j >= len(results[i].Failures) { - t.Errorf("results[%d] (%s): missing expected failure %q", i, tt.name, want) - continue - } - if !strings.Contains(results[i].Failures[j], want) { - t.Errorf("results[%d] (%s): failure[%d] = %q, want it to contain %q", i, tt.name, j, results[i].Failures[j], want) - } - } - } -} - -func TestRunEvalFile(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - file string - }{ - {name: "eval", file: "eval.star"}, - {name: "builtins", file: "builtins_test.star"}, - {name: "mock_fs", file: "mock_fs.star"}, - } - - ruleTypes, err := loadRulesFromDir("testdata") - if err != nil { - t.Fatalf("loading rule types: %v", err) - } - - r := NewRunner() - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - results, err := r.RunFile(filepath.Join("testdata", tt.file), nil, ruleTypes) - if err != nil { - t.Fatalf("RunFile failed for %s: %v", tt.file, err) - } - for _, res := range results { - if strings.HasPrefix(res.Name, "test_fail_") { - if res.Passed() { - t.Errorf("expected test %s to fail, but it passed", res.Name) - } - continue - } - if !res.Passed() { - t.Errorf("test %s failed: %v", res.Name, res.Failures) - } - } - }) - } -} - func TestTestDir(t *testing.T) { t.Parallel() r := NewRunner() - dir := t.TempDir() - content := []byte(` -def test_pass(): - assert.eq(1, 1) -`) - if err := os.WriteFile(filepath.Join(dir, "pass.star"), content, 0644); err != nil { - t.Fatal(err) - } - r.TestDir(t, dir) + r.TestDir(t, "testdata") } diff --git a/pkg/ruletest/testdata/sample.star b/pkg/ruletest/testdata/sample.star index 18bc8cc3d4..8052898e3a 100644 --- a/pkg/ruletest/testdata/sample.star +++ b/pkg/ruletest/testdata/sample.star @@ -3,11 +3,11 @@ def test_passing(): pass # Simple test that fails using assert.fail() -def test_failing(): +def test_fail_failing(): assert.fail("this test failed intentionally") # Test that throws a Starlark exception -def test_exception(): +def test_fail_exception(): 1 / 0 # A helper function, not a test (does not start with test_) From ff1ceff53cfdbfac1352133763909cdfa284cb0b Mon Sep 17 00:00:00 2001 From: krrish175-byte Date: Fri, 3 Jul 2026 01:31:43 +0530 Subject: [PATCH 6/8] feat: integrate filesystem mocking in ruletest eval --- pkg/ruletest/eval.go | 6 +++- pkg/ruletest/eval_test.go | 4 +-- pkg/testkit/v1/testkit_provider.go | 50 +++--------------------------- 3 files changed, 12 insertions(+), 48 deletions(-) diff --git a/pkg/ruletest/eval.go b/pkg/ruletest/eval.go index d60226e5a9..c77f01a247 100644 --- a/pkg/ruletest/eval.go +++ b/pkg/ruletest/eval.go @@ -77,6 +77,10 @@ func (tr *testCaseRunner) builtinEval( return nil, fmt.Errorf("failed to initialize rule type engine: %w", err) } + if tk.ShouldOverrideIngest() { + rte.WithCustomIngester(tk) + } + res, err := rte.Eval(ctx, entityProto, profileMap, nil, &stubResultSink{}) // Because Eval returns the error, we pass that error to formatEvalResult @@ -134,7 +138,7 @@ func parseMockFSDict(mockFSDict *starlark.Dict) (map[string]string, error) { func (tr *testCaseRunner) lookupRuleType(ruleName string) (*minderv1.RuleType, error) { if tr.ruleTypes != nil { - if ruleType, ok := tr.ruleTypes[ruleName]; ok { + if ruleType := tr.ruleTypes[ruleName]; ruleType != nil { return ruleType, nil } } diff --git a/pkg/ruletest/eval_test.go b/pkg/ruletest/eval_test.go index cbfbd9fbfb..437fef8e99 100644 --- a/pkg/ruletest/eval_test.go +++ b/pkg/ruletest/eval_test.go @@ -97,7 +97,7 @@ func TestBuiltinEval_InvalidArgs(t *testing.T) { }, { name: "invalid mock_fs type", - args: starlark.Tuple{starlark.String("rule.yaml")}, + args: starlark.Tuple{starlark.String("fs_check")}, kwargs: []starlark.Tuple{ {starlark.String("mock_fs"), starlark.String("not a dict")}, }, @@ -105,7 +105,7 @@ func TestBuiltinEval_InvalidArgs(t *testing.T) { }, { name: "mock_fs non-string keys", - args: starlark.Tuple{starlark.String("rule.yaml")}, + args: starlark.Tuple{starlark.String("fs_check")}, kwargs: []starlark.Tuple{ { starlark.String("mock_fs"), diff --git a/pkg/testkit/v1/testkit_provider.go b/pkg/testkit/v1/testkit_provider.go index 5aef0de3e6..a41d4783b7 100644 --- a/pkg/testkit/v1/testkit_provider.go +++ b/pkg/testkit/v1/testkit_provider.go @@ -6,11 +6,8 @@ package v1 import ( "context" "errors" - "time" "github.com/go-git/go-git/v5" - "github.com/go-git/go-git/v5/plumbing/object" - "github.com/go-git/go-git/v5/storage/memory" "google.golang.org/protobuf/reflect/protoreflect" minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" @@ -86,46 +83,9 @@ func (*TestKit) PropertiesToProtoMessage(_ minderv1.Entity, _ *properties.Proper return nil, nil } -// Clone Implements the Git trait. If gitFS is set, it initializes an -// in-memory Git repository backed by that filesystem. -func (tk *TestKit) Clone(_ context.Context, _ string, _ string) (*git.Repository, error) { - if tk.gitFS == nil { - return nil, ErrNotIngesterOverridden - } - - storer := memory.NewStorage() - - repo, err := git.Init(storer, tk.gitFS) - if err != nil { - return nil, err - } - - w, err := repo.Worktree() - if err != nil { - return nil, err - } - - // Add all files from the worktree - status, err := w.Status() - if err != nil { - return nil, err - } - for path := range status { - if _, err := w.Add(path); err != nil { - continue - } - } - - _, err = w.Commit("Initial commit", &git.CommitOptions{ - Author: &object.Signature{ - Name: "TestKit", - Email: "testkit@minder.test", - When: time.Now(), - }, - }) - if err != nil { - return nil, err - } - - return repo, nil +// Clone implements the Git interface. +// TestKit relies on fakeGit and Ingest for filesystem operations. +func (*TestKit) Clone(_ context.Context, _ string, _ string) (*git.Repository, error) { + return nil, errors.New("Clone is not supported in TestKit; use Ingest instead") } + From 19919910970b8e8dc375a8c3a8f21cb81f43d9ef Mon Sep 17 00:00:00 2001 From: krrish175-byte Date: Fri, 3 Jul 2026 13:24:30 +0530 Subject: [PATCH 7/8] chore: fix PR comments from latest review --- pkg/ruletest/runner.go | 44 ++++-------------------------- pkg/ruletest/runner_test.go | 36 +++++++++++++++++++++++- pkg/testkit/v1/testkit_provider.go | 1 - 3 files changed, 41 insertions(+), 40 deletions(-) diff --git a/pkg/ruletest/runner.go b/pkg/ruletest/runner.go index bb315ba5eb..a753806514 100644 --- a/pkg/ruletest/runner.go +++ b/pkg/ruletest/runner.go @@ -9,11 +9,12 @@ import ( "fmt" "io" "io/fs" + "maps" "os" "path/filepath" + "slices" "sort" "strings" - "testing" "go.starlark.net/starlark" "go.starlark.net/starlarktest" @@ -187,6 +188,9 @@ func loadRulesFromDir(dir string) (map[string]*minderv1.RuleType, error) { continue // skip files that aren't valid rule types } if rt != nil && rt.Name != "" { + if _, exists := ruleTypes[rt.Name]; exists { + return nil, fmt.Errorf("duplicate rule type name %q in directory %s", rt.Name, dir) + } ruleTypes[rt.Name] = rt } } @@ -232,11 +236,7 @@ func (r *Runner) RunPaths(paths []string) ([]TestResult, error) { var errs []error // Ensure deterministic execution order by sorting directories - var dirs []string - for dir := range filesByDir { - dirs = append(dirs, dir) - } - sort.Strings(dirs) + dirs := slices.Sorted(maps.Keys(filesByDir)) for _, dir := range dirs { files := filesByDir[dir] @@ -260,35 +260,3 @@ func (r *Runner) RunPaths(paths []string) ([]TestResult, error) { return allResults, errors.Join(errs...) } -// TestDir discovers and executes all *.star test files under the given -// directory, reporting results through t. It also loads *.yaml rules. -//nolint:tparallel // TestDir acts as a test runner helper, caller is responsible for t.Parallel -func (r *Runner) TestDir(t *testing.T, dir string) { - t.Helper() - - results, err := r.RunPaths([]string{dir}) - if err != nil { - t.Fatalf("running tests in %s: %v", dir, err) - } - - if len(results) == 0 { - t.Logf("no *.star test files found in %s", dir) - return - } - - for _, result := range results { - result := result - name := result.Filename + "/" + result.Name - t.Run(name, func(t *testing.T) { - if strings.HasPrefix(result.Name, "test_fail_") { - if result.Passed() { - t.Errorf("expected test %s to fail, but it passed", result.Name) - } - return - } - for _, msg := range result.Failures { - t.Error(msg) - } - }) - } -} diff --git a/pkg/ruletest/runner_test.go b/pkg/ruletest/runner_test.go index cafc6214e4..ba326ca4f9 100644 --- a/pkg/ruletest/runner_test.go +++ b/pkg/ruletest/runner_test.go @@ -4,11 +4,45 @@ package ruletest import ( + "strings" "testing" ) func TestTestDir(t *testing.T) { t.Parallel() r := NewRunner() - r.TestDir(t, "testdata") + testDir(t, r, "testdata") +} + +// testDir discovers and executes all *.star test files under the given +// directory, reporting results through t. It also loads *.yaml rules. +func testDir(t *testing.T, r *Runner, dir string) { + t.Helper() + + results, err := r.RunPaths([]string{dir}) + if err != nil { + t.Fatalf("running tests in %s: %v", dir, err) + } + + if len(results) == 0 { + t.Logf("no *.star test files found in %s", dir) + return + } + + for _, result := range results { + result := result + name := result.Filename + "/" + result.Name + t.Run(name, func(t *testing.T) { + t.Parallel() + if strings.HasPrefix(result.Name, "test_fail_") { + if result.Passed() { + t.Errorf("expected test %s to fail, but it passed", result.Name) + } + return + } + for _, msg := range result.Failures { + t.Error(msg) + } + }) + } } diff --git a/pkg/testkit/v1/testkit_provider.go b/pkg/testkit/v1/testkit_provider.go index a41d4783b7..5b45ece9f9 100644 --- a/pkg/testkit/v1/testkit_provider.go +++ b/pkg/testkit/v1/testkit_provider.go @@ -88,4 +88,3 @@ func (*TestKit) PropertiesToProtoMessage(_ minderv1.Entity, _ *properties.Proper func (*TestKit) Clone(_ context.Context, _ string, _ string) (*git.Repository, error) { return nil, errors.New("Clone is not supported in TestKit; use Ingest instead") } - From f7beb5a8ba7395266891453bc03e4e950b2de0e8 Mon Sep 17 00:00:00 2001 From: krrish175-byte Date: Fri, 3 Jul 2026 13:45:25 +0530 Subject: [PATCH 8/8] chore: fix golangci-lint errors (gci and revive) --- pkg/ruletest/eval.go | 2 +- pkg/ruletest/runner.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/ruletest/eval.go b/pkg/ruletest/eval.go index c77f01a247..7364033bbd 100644 --- a/pkg/ruletest/eval.go +++ b/pkg/ruletest/eval.go @@ -20,7 +20,7 @@ import ( ) func (tr *testCaseRunner) builtinEval( - thread *starlark.Thread, _ *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple, + _ *starlark.Thread, _ *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple, ) (starlark.Value, error) { var ruleName string var entityDict *starlark.Dict diff --git a/pkg/ruletest/runner.go b/pkg/ruletest/runner.go index a753806514..718a651339 100644 --- a/pkg/ruletest/runner.go +++ b/pkg/ruletest/runner.go @@ -259,4 +259,3 @@ func (r *Runner) RunPaths(paths []string) ([]TestResult, error) { } return allResults, errors.Join(errs...) } -