Skip to content

Commit e327c0e

Browse files
Add JSON schema validation for expected_profile.json
Reject invalid/empty JSON files that were silently passing tests. Validates: - stacks required (unless note field explains why empty) - profile-type and stack-content required per stack - regular_expression required per stack-content entry - value or percent required (unless parent has value-matching-sum) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 1cacbd0 commit e327c0e

File tree

5 files changed

+278
-39
lines changed

5 files changed

+278
-39
lines changed

go.mod

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,9 @@ require (
77
github.com/klauspost/compress v1.17.11
88
github.com/pierrec/lz4/v4 v4.1.21
99
)
10+
11+
require (
12+
github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f // indirect
13+
github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect
14+
github.com/xeipuuv/gojsonschema v1.2.0 // indirect
15+
)

go.sum

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,16 @@
1+
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
12
github.com/google/pprof v0.0.0-20240528025155-186aa0362fba h1:ql1qNgCyOB7iAEk8JTNM+zJrgIbnyCKX/wdlyPufP5g=
23
github.com/google/pprof v0.0.0-20240528025155-186aa0362fba/go.mod h1:K1liHPHnj73Fdn/EKuT8nrFqBihUSKXoLYU0BuatOYo=
34
github.com/klauspost/compress v1.17.11 h1:In6xLpyWOi1+C7tXUUWv2ot1QvBjxevKAaI6IXrJmUc=
45
github.com/klauspost/compress v1.17.11/go.mod h1:pMDklpSncoRMuLFrf1W9Ss9KT+0rH90U12bZKk7uwG0=
56
github.com/pierrec/lz4/v4 v4.1.21 h1:yOVMLb6qSIDP67pl/5F7RepeKYu/VmTyEXvuMI5d9mQ=
67
github.com/pierrec/lz4/v4 v4.1.21/go.mod h1:gZWDp/Ze/IJXGXf23ltt2EXimqmTUXEy0GFuRQyBid4=
8+
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
9+
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
10+
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
11+
github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f h1:J9EGpcZtP0E/raorCMxlFGSTBrsSlaDGf3jU/qvAE2c=
12+
github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU=
13+
github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 h1:EzJWgHovont7NscjpAxXsDA8S8BMYve8Y5+7cuRE7R0=
14+
github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415/go.mod h1:GwrjFmJcFw6At/Gs6z4yjiIwzuJ1/+UwLxMQDVQXShQ=
15+
github.com/xeipuuv/gojsonschema v1.2.0 h1:LhYJRs+L4fBtjZUfuSZIKGeVu0QRy8e5Xi7D17UxZ74=
16+
github.com/xeipuuv/gojsonschema v1.2.0/go.mod h1:anYRn/JVcOK2ZgGU+IjEV4nwlhoK5sQluxsYJ78Id3Y=

pprof_analysis.go

Lines changed: 95 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,57 @@ import (
1717
"github.com/google/pprof/profile"
1818
"github.com/klauspost/compress/zstd"
1919
"github.com/pierrec/lz4/v4"
20+
"github.com/xeipuuv/gojsonschema"
2021
)
2122

2223
var (
2324
_ json.Unmarshaler = (*Optional[int64])(nil)
2425
_ json.Marshaler = (*Optional[int64])(nil)
2526
)
2627

28+
// JSON Schema for validating expected_profile.json files
29+
// Basic structure validation - complex business rules validated in Go code
30+
var expectedProfileSchema = `{
31+
"$schema": "https://json-schema.org/draft-07/schema#",
32+
"type": "object",
33+
"required": ["stacks"],
34+
"properties": {
35+
"test_name": { "type": "string" },
36+
"note": { "type": "string" },
37+
"scale_by_duration": { "type": "boolean" },
38+
"pprof-regex": { "type": "string" },
39+
"allow_first_profile_failure": { "type": "boolean" },
40+
"stacks": {
41+
"type": "array",
42+
"items": {
43+
"type": "object",
44+
"required": ["profile-type", "stack-content"],
45+
"properties": {
46+
"profile-type": { "type": "string", "minLength": 1 },
47+
"pprof-regex": { "type": "string" },
48+
"stack-content": {
49+
"type": "array",
50+
"minItems": 1,
51+
"items": {
52+
"type": "object",
53+
"required": ["regular_expression"],
54+
"properties": {
55+
"regular_expression": { "type": "string", "minLength": 1 },
56+
"value": { "type": "integer" },
57+
"percent": { "type": "integer" },
58+
"error_margin": { "type": "integer" },
59+
"labels": { "type": "array" }
60+
}
61+
}
62+
},
63+
"error-margin": { "type": "integer" },
64+
"value-matching-sum": { "type": "integer" }
65+
}
66+
}
67+
}
68+
}
69+
}`
70+
2771
type Optional[T any] struct {
2872
value *T
2973
}
@@ -94,12 +138,37 @@ type TypedStacks struct {
94138

95139
type StackTestData struct {
96140
TestName string `json:"test_name"`
141+
Note string `json:"note,omitempty"`
97142
ScaleByDuration bool `json:"scale_by_duration"`
98143
PprofRegex string `json:"pprof-regex"`
99144
AllowFirstProfileFailure bool `json:"allow_first_profile_failure,omitempty"`
100145
Stacks []TypedStacks `json:"stacks"`
101146
}
102147

148+
// Validate rules that JSON Schema can't express
149+
func (s *StackTestData) Validate() error {
150+
// Stacks must be non-empty unless note is present
151+
if len(s.Stacks) == 0 && s.Note == "" {
152+
return fmt.Errorf("'stacks' must have at least one entry (or provide a 'note' explaining why it's empty)")
153+
}
154+
155+
// If no value-matching-sum, require value or percent in stack-content
156+
for i, stack := range s.Stacks {
157+
if _, hasValueMatchingSum := stack.ValueMatchingSum.Value(); hasValueMatchingSum {
158+
continue
159+
}
160+
for j, content := range stack.StackContent {
161+
_, hasValue := content.Value.Value()
162+
_, hasPercent := content.Percent.Value()
163+
if !hasValue && !hasPercent {
164+
return fmt.Errorf("stacks[%d].stack-content[%d]: must have 'value' or 'percent' (or parent must have 'value-matching-sum')", i, j)
165+
}
166+
}
167+
}
168+
169+
return nil
170+
}
171+
103172
// Custom unmarshaller for Labels to ensure exactly one of Values and ValueRegex is defined
104173
func (l *Labels) UnmarshalJSON(data []byte) error {
105174
type labels Labels
@@ -442,21 +511,41 @@ func writeToJSONFile(data StackTestData, filePath string) error {
442511

443512
func readJSONFile(filePath string) (StackTestData, error) {
444513
var data StackTestData
445-
jsonFile, err := os.Open(filePath)
514+
byteValue, err := os.ReadFile(filePath)
446515
if err != nil {
447516
return data, err
448517
}
449-
defer jsonFile.Close()
450-
byteValue, err := io.ReadAll(jsonFile)
518+
519+
// Step 1: Validate JSON syntax
520+
if !json.Valid(byteValue) {
521+
return data, fmt.Errorf("invalid JSON syntax in %s", filePath)
522+
}
523+
524+
// Step 2: Validate against schema
525+
schemaLoader := gojsonschema.NewStringLoader(expectedProfileSchema)
526+
documentLoader := gojsonschema.NewBytesLoader(byteValue)
527+
result, err := gojsonschema.Validate(schemaLoader, documentLoader)
451528
if err != nil {
452-
return data, err
529+
return data, fmt.Errorf("schema validation error for %s: %v", filePath, err)
453530
}
454-
if !json.Valid(byteValue) {
455-
return data, fmt.Errorf("Invalid json data %s", filePath)
531+
if !result.Valid() {
532+
var errs []string
533+
for _, desc := range result.Errors() {
534+
errs = append(errs, desc.String())
535+
}
536+
return data, fmt.Errorf("JSON schema validation failed for %s:\n - %s", filePath, strings.Join(errs, "\n - "))
456537
}
538+
539+
// Step 3: Unmarshal validated JSON
457540
if err := json.Unmarshal(byteValue, &data); err != nil {
458541
return data, err
459542
}
543+
544+
// Step 4: Validate business rules
545+
if err := data.Validate(); err != nil {
546+
return data, fmt.Errorf("validation failed for %s: %v", filePath, err)
547+
}
548+
460549
return data, nil
461550
}
462551

scenarios/dotnet_wall/expected_profile.json

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -35,39 +35,6 @@
3535
]
3636
}
3737
]
38-
},
39-
{
40-
"profile-type": "cpu",
41-
"pprof-regex": "",
42-
"stack-content": null,
43-
"error-margin": 3,
44-
"value-matching-sum": null
45-
},
46-
{
47-
"profile-type": "exception",
48-
"pprof-regex": "",
49-
"stack-content": null,
50-
"error-margin": 3,
51-
"value-matching-sum": null
52-
},
53-
{
54-
"profile-type": "lock-count",
55-
"pprof-regex": "",
56-
"stack-content": null,
57-
"error-margin": 3,
58-
"value-matching-sum": null
59-
},
60-
{
61-
"profile-type": "lock-time",
62-
"pprof-regex": "",
63-
"stack-content": null
64-
},
65-
{
66-
"profile-type": "timeline",
67-
"pprof-regex": "",
68-
"stack-content": null,
69-
"error-margin": 3,
70-
"value-matching-sum": null
7138
}
7239
],
7340
"scale_by_duration": false

schema_test.go

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
package main
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"strings"
7+
"testing"
8+
)
9+
10+
func TestSchemaValidation(t *testing.T) {
11+
tests := []struct {
12+
name string
13+
content string
14+
wantErr bool
15+
errContains string
16+
}{
17+
{
18+
name: "empty JSON",
19+
content: `{}`,
20+
wantErr: true,
21+
errContains: "stacks is required",
22+
},
23+
{
24+
name: "wrong structure",
25+
content: `{"foo": "bar"}`,
26+
wantErr: true,
27+
errContains: "stacks is required",
28+
},
29+
{
30+
name: "empty stacks without note",
31+
content: `{"stacks": []}`,
32+
wantErr: true,
33+
errContains: "stacks",
34+
},
35+
{
36+
name: "empty stacks with note",
37+
content: `{"stacks": [], "note": "This test doesn't check profile content"}`,
38+
wantErr: false,
39+
},
40+
{
41+
name: "missing profile-type",
42+
content: `{"stacks": [{"stack-content": [{"regular_expression": "test", "value": 100}]}]}`,
43+
wantErr: true,
44+
errContains: "profile-type",
45+
},
46+
{
47+
name: "missing stack-content",
48+
content: `{"stacks": [{"profile-type": "wall-time"}]}`,
49+
wantErr: true,
50+
errContains: "stack-content",
51+
},
52+
{
53+
name: "empty stack-content",
54+
content: `{"stacks": [{"profile-type": "wall-time", "stack-content": []}]}`,
55+
wantErr: true,
56+
errContains: "stack-content",
57+
},
58+
{
59+
name: "missing regular_expression",
60+
content: `{"stacks": [{"profile-type": "wall-time", "stack-content": [{"value": 100}]}]}`,
61+
wantErr: true,
62+
errContains: "regular_expression",
63+
},
64+
{
65+
name: "missing value and percent",
66+
content: `{"stacks": [{"profile-type": "wall-time", "stack-content": [{"regular_expression": "test"}]}]}`,
67+
wantErr: true,
68+
errContains: "value",
69+
},
70+
{
71+
name: "no value/percent but has value-matching-sum",
72+
content: `{
73+
"stacks": [{
74+
"profile-type": "cpu-time",
75+
"stack-content": [{"regular_expression": ".*test.*"}],
76+
"value-matching-sum": 1000000000
77+
}]
78+
}`,
79+
wantErr: false,
80+
},
81+
{
82+
name: "valid JSON with value",
83+
content: `{
84+
"test_name": "test",
85+
"stacks": [{
86+
"profile-type": "wall-time",
87+
"stack-content": [{"regular_expression": "^test$", "value": 100}]
88+
}]
89+
}`,
90+
wantErr: false,
91+
},
92+
{
93+
name: "valid JSON with percent",
94+
content: `{
95+
"stacks": [{
96+
"profile-type": "wall-time",
97+
"stack-content": [{"regular_expression": "^test$", "percent": 50}]
98+
}]
99+
}`,
100+
wantErr: false,
101+
},
102+
}
103+
104+
for _, tt := range tests {
105+
t.Run(tt.name, func(t *testing.T) {
106+
tmpFile, err := os.CreateTemp("", "schema_test_*.json")
107+
if err != nil {
108+
t.Fatalf("Failed to create temp file: %v", err)
109+
}
110+
defer os.Remove(tmpFile.Name())
111+
112+
if _, err := tmpFile.WriteString(tt.content); err != nil {
113+
t.Fatalf("Failed to write to temp file: %v", err)
114+
}
115+
tmpFile.Close()
116+
117+
_, err = readJSONFile(tmpFile.Name())
118+
119+
if tt.wantErr {
120+
if err == nil {
121+
t.Errorf("Expected error containing %q, got nil", tt.errContains)
122+
} else if !strings.Contains(err.Error(), tt.errContains) {
123+
t.Errorf("Expected error containing %q, got: %v", tt.errContains, err)
124+
}
125+
} else {
126+
if err != nil {
127+
t.Errorf("Expected success, got error: %v", err)
128+
}
129+
}
130+
})
131+
}
132+
}
133+
134+
func TestSchemaValidation_AllExistingProfiles(t *testing.T) {
135+
scenariosDir := "scenarios"
136+
if _, err := os.Stat(scenariosDir); os.IsNotExist(err) {
137+
t.Skip("scenarios directory not found")
138+
}
139+
140+
var jsonFiles []string
141+
err := filepath.Walk(scenariosDir, func(path string, info os.FileInfo, err error) error {
142+
if err != nil {
143+
return err
144+
}
145+
if info.Name() == "expected_profile.json" {
146+
jsonFiles = append(jsonFiles, path)
147+
}
148+
return nil
149+
})
150+
if err != nil {
151+
t.Fatalf("Failed to walk scenarios directory: %v", err)
152+
}
153+
154+
if len(jsonFiles) == 0 {
155+
t.Skip("No expected_profile.json files found")
156+
}
157+
158+
t.Logf("Found %d expected_profile.json files to validate", len(jsonFiles))
159+
160+
for _, jsonFile := range jsonFiles {
161+
t.Run(jsonFile, func(t *testing.T) {
162+
if _, err := readJSONFile(jsonFile); err != nil {
163+
t.Errorf("Validation failed: %v", err)
164+
}
165+
})
166+
}
167+
}

0 commit comments

Comments
 (0)