Skip to content

Commit 673806c

Browse files
NoNickeDNoNickeD
committed
refactor: rename group flag to file-name-override and improve tests
 - Renamed flag from 'group' to 'file-name-override' for clarity - Rewrote tests to use TestRunner.Run() method with proper test fixtures - Added comprehensive acceptance tests for the new flag - Fixed Rego v1 syntax in test policies - Added proper error handling and edge case testing Addresses review feedback from PR open-policy-agent#1163 Signed-off-by: nikos.nikolakakis <[email protected]>
1 parent 9f2b39f commit 673806c

File tree

4 files changed

+161
-53
lines changed

4 files changed

+161
-53
lines changed

‎acceptance.bats‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -530,3 +530,30 @@ EOF"
530530
[ "$status" -eq 1 ]
531531
[[ "$output" =~ "10 tests, 3 passed, 0 warnings, 7 failures, 0 exceptions" ]]
532532
}
533+
534+
@test "File name override flag replaces stdin filename in output" {
535+
run bash -c "./conftest test --file-name-override='my-custom-file.yaml' -p examples/kubernetes/policy - < examples/kubernetes/service.yaml"
536+
[ "$status" -eq 0 ]
537+
[[ "$output" =~ "my-custom-file.yaml" ]]
538+
[[ ! "$output" =~ "-" ]]
539+
}
540+
541+
@test "File name override flag does not affect regular files" {
542+
run ./conftest test --file-name-override='override.yaml' -p examples/kubernetes/policy examples/kubernetes/service.yaml
543+
[ "$status" -eq 0 ]
544+
[[ "$output" =~ "examples/kubernetes/service.yaml" ]]
545+
[[ ! "$output" =~ "override.yaml" ]]
546+
}
547+
548+
@test "File name override flag works with JSON output" {
549+
run bash -c "./conftest test --file-name-override='custom.json' --output json -p examples/kubernetes/policy - < examples/kubernetes/service.yaml"
550+
[ "$status" -eq 0 ]
551+
[[ "$output" =~ "\"filename\":\"custom.json\"" ]]
552+
[[ ! "$output" =~ "\"filename\":\"-\"" ]]
553+
}
554+
555+
@test "Without file name override flag, stdin shows as dash" {
556+
run bash -c "./conftest test -p examples/kubernetes/policy - < examples/kubernetes/service.yaml"
557+
[ "$status" -eq 0 ]
558+
[[ "$output" =~ "-" ]]
559+
}

‎internal/commands/test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ func NewTestCommand(ctx context.Context) *cobra.Command {
115115
"junit-hide-message",
116116
"quiet",
117117
"tls",
118-
"group",
118+
"file-name-override",
119119
}
120120
for _, name := range flagNames {
121121
if err := viper.BindPFlag(name, cmd.Flags().Lookup(name)); err != nil {
@@ -197,7 +197,7 @@ func NewTestCommand(ctx context.Context) *cobra.Command {
197197

198198
cmd.Flags().StringSlice("proto-file-dirs", []string{}, "A list of directories containing Protocol Buffer definitions")
199199
cmd.Flags().Bool("tls", true, "Use TLS to access the registry")
200-
cmd.Flags().String("group", "", "Override the group name for stdin input (used for output formatting)")
200+
cmd.Flags().String("file-name-override", "", "Override the file name for stdin input (used for output formatting)")
201201

202202
return &cmd
203203
}

‎runner/test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ type TestRunner struct {
3535
Combine bool
3636
Quiet bool
3737
Output string
38-
Group string
38+
FileNameOverride string `mapstructure:"file-name-override"`
3939
}
4040

4141
// Run executes the TestRunner, verifying all Rego policies against the given
@@ -111,11 +111,11 @@ func (t *TestRunner) Run(ctx context.Context, fileList []string) (output.CheckRe
111111
}
112112
}
113113

114-
// Override group name for stdin input if --group flag is provided
115-
if t.Group != "" {
114+
// Override file name for stdin input if --file-name-override flag is provided
115+
if t.FileNameOverride != "" {
116116
for i := range results {
117117
if results[i].FileName == "-" {
118-
results[i].FileName = t.Group
118+
results[i].FileName = t.FileNameOverride
119119
}
120120
}
121121
}

‎runner/test_test.go‎

Lines changed: 128 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,77 +1,158 @@
11
package runner
22

33
import (
4+
"context"
5+
"os"
6+
"path/filepath"
47
"testing"
5-
6-
"github.com/open-policy-agent/conftest/output"
78
)
89

9-
func TestTestRunner_GroupFlag(t *testing.T) {
10-
// Create a mock TestRunner with Group set
11-
runner := TestRunner{
12-
Group: "my-custom-group",
10+
func TestTestRunner_FileNameOverrideFlag(t *testing.T) {
11+
// Create a temporary directory for test files
12+
tmpDir := t.TempDir()
13+
14+
// Create a test policy file
15+
policyDir := filepath.Join(tmpDir, "policy")
16+
if err := os.Mkdir(policyDir, 0755); err != nil {
17+
t.Fatalf("Failed to create policy directory: %v", err)
18+
}
19+
20+
policyContent := `package main
21+
22+
deny contains msg if {
23+
input.kind == "Deployment"
24+
input.metadata.name == "test"
25+
msg := "test deployment found"
26+
}
27+
`
28+
policyFile := filepath.Join(policyDir, "test.rego")
29+
if err := os.WriteFile(policyFile, []byte(policyContent), 0644); err != nil {
30+
t.Fatalf("Failed to write policy file: %v", err)
1331
}
1432

15-
// Create mock results with stdin filename
16-
results := output.CheckResults{
33+
// Test cases
34+
tests := []struct {
35+
name string
36+
fileNameOverride string
37+
expectedFileName string
38+
}{
1739
{
18-
FileName: "-",
19-
Namespace: "main",
20-
Failures: []output.Result{{Message: "test failure"}},
40+
name: "with file-name-override",
41+
fileNameOverride: "my-custom-file.yaml",
42+
expectedFileName: "my-custom-file.yaml",
2143
},
2244
{
23-
FileName: "regular-file.yaml",
24-
Namespace: "main",
25-
Failures: []output.Result{{Message: "another failure"}},
45+
name: "without file-name-override",
46+
fileNameOverride: "",
47+
expectedFileName: "-",
2648
},
2749
}
2850

29-
// Test the group name override logic
30-
if runner.Group != "" {
31-
for i := range results {
32-
if results[i].FileName == "-" {
33-
results[i].FileName = runner.Group
51+
for _, tc := range tests {
52+
t.Run(tc.name, func(t *testing.T) {
53+
// Create TestRunner with FileNameOverride set
54+
runner := TestRunner{
55+
Policy: []string{policyDir},
56+
Namespace: []string{"main"},
57+
FileNameOverride: tc.fileNameOverride,
58+
RegoVersion: "v1",
3459
}
35-
}
60+
61+
// Create stdin input by using "-" as the file path
62+
fileList := []string{"-"}
63+
64+
// Mock stdin with test data
65+
oldStdin := os.Stdin
66+
r, w, _ := os.Pipe()
67+
os.Stdin = r
68+
testInput := `{
69+
"apiVersion": "apps/v1",
70+
"kind": "Deployment",
71+
"metadata": {
72+
"name": "test"
3673
}
74+
}`
75+
go func() {
76+
defer w.Close()
77+
w.Write([]byte(testInput))
78+
}()
79+
defer func() { os.Stdin = oldStdin }()
80+
81+
// Run the test
82+
ctx := context.Background()
83+
results, err := runner.Run(ctx, fileList)
84+
if err != nil {
85+
t.Fatalf("Run failed: %v", err)
86+
}
87+
88+
// Verify results
89+
if len(results) == 0 {
90+
t.Fatal("Expected at least one result")
91+
}
3792

38-
// Verify the stdin result was updated
39-
if results[0].FileName != "my-custom-group" {
40-
t.Errorf("Expected stdin filename to be overridden to 'my-custom-group', got '%s'", results[0].FileName)
93+
// Check that the filename was properly overridden
94+
if results[0].FileName != tc.expectedFileName {
95+
t.Errorf("Expected filename to be '%s', got '%s'", tc.expectedFileName, results[0].FileName)
96+
}
97+
})
4198
}
99+
}
100+
101+
func TestTestRunner_FileNameOverrideOnlyAffectsStdin(t *testing.T) {
102+
// Create a temporary directory for test files
103+
tmpDir := t.TempDir()
42104

43-
// Verify the regular file result was not changed
44-
if results[1].FileName != "regular-file.yaml" {
45-
t.Errorf("Expected regular filename to remain 'regular-file.yaml', got '%s'", results[1].FileName)
105+
// Create a test policy file
106+
policyDir := filepath.Join(tmpDir, "policy")
107+
if err := os.Mkdir(policyDir, 0755); err != nil {
108+
t.Fatalf("Failed to create policy directory: %v", err)
46109
}
110+
111+
policyContent := `package main
112+
113+
deny contains msg if {
114+
input.kind == "Deployment"
115+
msg := "deployment found"
47116
}
117+
`
118+
policyFile := filepath.Join(policyDir, "test.rego")
119+
if err := os.WriteFile(policyFile, []byte(policyContent), 0644); err != nil {
120+
t.Fatalf("Failed to write policy file: %v", err)
121+
}
48122

49-
func TestTestRunner_GroupFlagEmpty(t *testing.T) {
50-
// Create a mock TestRunner without Group set
123+
// Create a test config file
124+
configContent := `apiVersion: apps/v1
125+
kind: Deployment
126+
metadata:
127+
name: test-file
128+
`
129+
configFile := filepath.Join(tmpDir, "deployment.yaml")
130+
if err := os.WriteFile(configFile, []byte(configContent), 0644); err != nil {
131+
t.Fatalf("Failed to write config file: %v", err)
132+
}
133+
134+
// Create TestRunner with FileNameOverride set
51135
runner := TestRunner{
52-
Group: "",
136+
Policy: []string{policyDir},
137+
Namespace: []string{"main"},
138+
FileNameOverride: "overridden-name.yaml",
139+
RegoVersion: "v1",
53140
}
54141

55-
// Create mock results with stdin filename
56-
results := output.CheckResults{
57-
{
58-
FileName: "-",
59-
Namespace: "main",
60-
Failures: []output.Result{{Message: "test failure"}},
61-
},
142+
// Run with a regular file (not stdin)
143+
ctx := context.Background()
144+
results, err := runner.Run(ctx, []string{configFile})
145+
if err != nil {
146+
t.Fatalf("Run failed: %v", err)
62147
}
63148

64-
// Test the group name override logic
65-
if runner.Group != "" {
66-
for i := range results {
67-
if results[i].FileName == "-" {
68-
results[i].FileName = runner.Group
69-
}
70-
}
149+
// Verify results
150+
if len(results) == 0 {
151+
t.Fatal("Expected at least one result")
71152
}
72153

73-
// Verify the stdin result was NOT updated
74-
if results[0].FileName != "-" {
75-
t.Errorf("Expected stdin filename to remain '-', got '%s'", results[0].FileName)
154+
// Check that the regular file name was NOT overridden
155+
if results[0].FileName != configFile {
156+
t.Errorf("Expected filename to remain '%s', got '%s'", configFile, results[0].FileName)
76157
}
77-
}
158+
}

0 commit comments

Comments
 (0)