diff --git a/.agents/rules/go-coding-rule.md b/.agents/rules/go-coding-rule.md index 9d2cd6e52..8395a3ba5 100644 --- a/.agents/rules/go-coding-rule.md +++ b/.agents/rules/go-coding-rule.md @@ -25,6 +25,10 @@ When developing or modifying Go code in the KHI project, you **must** adhere to - Add GoDoc comments for private types, functions, and methods when their names are not self-explanatory or usage is not intuitive. - Add `var _ Interface = (*Implementation)(nil);` after the type definition to show that it's implementing the interface explicitly. +2. **Error and Test Assertion Messages**: + - Do not capitalize the first character of messages in `fmt.Errorf`, `t.Error`, `t.Errorf`, `t.Fatal`, `t.Fatalf`, etc. (e.g., use lowercase "failed to ..." instead of "Failed to ..."). + - This rule does not apply if the message starts with a capitalized name, such as a method name or proper acronym (e.g. `MyFunction() mismatch (-want +got)`). + ## Testing Practices 1. **Table-Driven Tests**: Tests must be written using the table-driven testing pattern. Define a slice of anonymous structs representing the test cases, and iterate over them using `t.Run()`. diff --git a/pkg/core/init/default/defaultextension.go b/pkg/core/init/default/defaultextension.go index d27457eee..430040bc3 100644 --- a/pkg/core/init/default/defaultextension.go +++ b/pkg/core/init/default/defaultextension.go @@ -30,6 +30,7 @@ import ( "github.com/GoogleCloudPlatform/khi/pkg/core/inspection/tracing" "github.com/GoogleCloudPlatform/khi/pkg/generated" "github.com/GoogleCloudPlatform/khi/pkg/model/k8s" + "github.com/GoogleCloudPlatform/khi/pkg/model/khifile/v6/style" "github.com/GoogleCloudPlatform/khi/pkg/parameters" "github.com/GoogleCloudPlatform/khi/pkg/server" "github.com/GoogleCloudPlatform/khi/pkg/server/option" @@ -147,6 +148,7 @@ func (d *DefaultInitExtension) ConfigureInspectionTaskServer(taskServer *coreins return err } } + style.LockRegistry() if *parameters.Auth.QuotaProjectID != "" { taskServer.AddRunContextOption(coreinspection.RunContextOptionArrayElementFromValue(googlecloudcommon_contract.APIClientFactoryOptionsContextKey, options.QuotaProject(*parameters.Auth.QuotaProjectID))) } diff --git a/pkg/model/khifile/v6/style/colorutil.go b/pkg/model/khifile/v6/style/colorutil.go new file mode 100644 index 000000000..2154be363 --- /dev/null +++ b/pkg/model/khifile/v6/style/colorutil.go @@ -0,0 +1,65 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package style + +import ( + "strconv" +) + +// ColorWhite represents the white color (1.0, 1.0, 1.0, 1.0). +var ColorWhite = Color{R: 1, G: 1, B: 1, A: 1} + +// ColorBlack represents the black color (0.0, 0.0, 0.0, 1.0). +var ColorBlack = Color{R: 0, G: 0, B: 0, A: 1} + +// MustForceConvertSRGBHex converts an sRGB hex color string to a Color. +// +// Deprecated: +// The timeline style uses the display-p3 color space. Directly mapping sRGB +// hex values to the 0.0-1.0 range in display-p3 is mathematically incorrect. +// Ideally, you should define a Color struct directly with proper display-p3 values. +func MustForceConvertSRGBHex(hex string) Color { + if len(hex) != 7 && len(hex) != 9 { + panic("invalid hex color length: " + hex) + } + if hex[0] != '#' { + panic("invalid hex color format (must start with '#'): " + hex) + } + r, err := strconv.ParseInt(hex[1:3], 16, 32) + if err != nil { + panic(err) + } + g, err := strconv.ParseInt(hex[3:5], 16, 32) + if err != nil { + panic(err) + } + b, err := strconv.ParseInt(hex[5:7], 16, 32) + if err != nil { + panic(err) + } + var a int64 = 255 + if len(hex) == 9 { + a, err = strconv.ParseInt(hex[7:9], 16, 32) + if err != nil { + panic(err) + } + } + return Color{ + R: float32(r) / 255.0, + G: float32(g) / 255.0, + B: float32(b) / 255.0, + A: float32(a) / 255.0, + } +} diff --git a/pkg/model/khifile/v6/style/colorutil_test.go b/pkg/model/khifile/v6/style/colorutil_test.go new file mode 100644 index 000000000..ff7ca6596 --- /dev/null +++ b/pkg/model/khifile/v6/style/colorutil_test.go @@ -0,0 +1,74 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package style + +import ( + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestMustForceConvertSRGBHex(t *testing.T) { + testCases := []struct { + name string + hex string + want Color + wantPanic bool + }{ + { + name: "valid 7-char hex", + hex: "#CC33CC", + want: Color{R: 204.0 / 255.0, G: 51.0 / 255.0, B: 204.0 / 255.0, A: 1.0}, + }, + { + name: "valid 9-char hex with alpha", + hex: "#0000FF80", + want: Color{R: 0.0, G: 0.0, B: 1.0, A: 128.0 / 255.0}, + }, + { + name: "invalid length 5-char", + hex: "#FFFF", + wantPanic: true, + }, + { + name: "invalid characters", + hex: "#GGGGGG", + wantPanic: true, + }, + { + name: "missing hash prefix", + hex: "CC33CC", + wantPanic: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + if tc.wantPanic { + defer func() { + if r := recover(); r == nil { + t.Errorf("expected panic, but did not panic") + } + }() + } + got := MustForceConvertSRGBHex(tc.hex) + if !tc.wantPanic { + if diff := cmp.Diff(tc.want, got); diff != "" { + t.Errorf("MustForceConvertSRGBHex() mismatch (-want +got):\n%s", diff) + } + } + }) + } +} diff --git a/pkg/model/khifile/v6/style/timeline.go b/pkg/model/khifile/v6/style/timeline.go index 3cc08cb88..4192d5d38 100644 --- a/pkg/model/khifile/v6/style/timeline.go +++ b/pkg/model/khifile/v6/style/timeline.go @@ -17,6 +17,7 @@ package style import ( _ "embed" "encoding/json" + "fmt" "slices" "sync" @@ -58,6 +59,7 @@ func GetIconAtlas() *pb.IconAtlas { var ( mu sync.RWMutex + locked bool severities []*pb.Severity verbs []*pb.Verb logTypes []*pb.LogType @@ -75,6 +77,15 @@ func reset() { logTypes = nil revisionStates = nil timelineTypes = nil + locked = false +} + +// LockRegistry locks the timeline style registry, preventing any further style registrations. +// This must be called after task registration has completed. +func LockRegistry() { + mu.Lock() + defer mu.Unlock() + locked = true } // Color represents a color with high dynamic range (HDR) capabilities. @@ -82,6 +93,24 @@ type Color struct { R, G, B, A float32 } +// Verify checks if R, G, B, and A are within [0.0, 1.0] range. +// Returns an error if any channel value is invalid. +func (c Color) Verify() error { + if c.R < 0 || c.R > 1 { + return fmt.Errorf("R channel %f is out of range [0, 1]", c.R) + } + if c.G < 0 || c.G > 1 { + return fmt.Errorf("G channel %f is out of range [0, 1]", c.G) + } + if c.B < 0 || c.B > 1 { + return fmt.Errorf("B channel %f is out of range [0, 1]", c.B) + } + if c.A < 0 || c.A > 1 { + return fmt.Errorf("A channel %f is out of range [0, 1]", c.A) + } + return nil +} + func (c Color) toProto() *pb.HDRColor4 { return &pb.HDRColor4{ R: proto.Float32(c.R), @@ -91,12 +120,22 @@ func (c Color) toProto() *pb.HDRColor4 { } } -// RegisterTimelineType registers a TimelineType, assigns a unique ID to it, +// MustRegisterTimelineType registers a TimelineType, assigns a unique ID to it, // and returns the generated pointer. This allows for global inline initialization in plugins. -func RegisterTimelineType(label string, description string, backgroundColor Color, foregroundColor Color, visible bool, sortPriority int32) *pb.TimelineType { +func MustRegisterTimelineType(label string, description string, backgroundColor Color, foregroundColor Color, visible bool, sortPriority int32) *pb.TimelineType { + if err := backgroundColor.Verify(); err != nil { + panic(fmt.Sprintf("invalid background color for timeline type %q: %v", label, err)) + } + if err := foregroundColor.Verify(); err != nil { + panic(fmt.Sprintf("invalid foreground color for timeline type %q: %v", label, err)) + } mu.Lock() defer mu.Unlock() + if locked { + panic(fmt.Sprintf("failed to register timeline type style %q: style-related registrations must be done in task/inspection/**/contract packages during package initialization. Did you call it from outside of the contract or not at package initialization timing?", label)) + } + id := uint32(len(timelineTypes) + 1) t := &pb.TimelineType{ Id: proto.Uint32(id), @@ -111,12 +150,22 @@ func RegisterTimelineType(label string, description string, backgroundColor Colo return t } -// RegisterSeverity registers a Severity, assigns a unique ID to it, +// MustRegisterSeverity registers a Severity, assigns a unique ID to it, // and returns the generated pointer. -func RegisterSeverity(label string, shortLabel string, backgroundColor Color, foregroundColor Color, order int32) *pb.Severity { +func MustRegisterSeverity(label string, shortLabel string, backgroundColor Color, foregroundColor Color, order int32) *pb.Severity { + if err := backgroundColor.Verify(); err != nil { + panic(fmt.Sprintf("invalid background color for severity %q: %v", label, err)) + } + if err := foregroundColor.Verify(); err != nil { + panic(fmt.Sprintf("invalid foreground color for severity %q: %v", label, err)) + } mu.Lock() defer mu.Unlock() + if locked { + panic(fmt.Sprintf("failed to register severity style %q: style-related registrations must be done in task/inspection/**/contract packages during package initialization. Did you call it from outside of the contract or not at package initialization timing?", label)) + } + id := uint32(len(severities) + 1) s := &pb.Severity{ Id: proto.Uint32(id), @@ -130,12 +179,22 @@ func RegisterSeverity(label string, shortLabel string, backgroundColor Color, fo return s } -// RegisterVerb registers a Verb, assigns a unique ID to it, +// MustRegisterVerb registers a Verb, assigns a unique ID to it, // and returns the generated pointer. -func RegisterVerb(label string, backgroundColor Color, foregroundColor Color, visible bool) *pb.Verb { +func MustRegisterVerb(label string, backgroundColor Color, foregroundColor Color, visible bool) *pb.Verb { + if err := backgroundColor.Verify(); err != nil { + panic(fmt.Sprintf("invalid background color for verb %q: %v", label, err)) + } + if err := foregroundColor.Verify(); err != nil { + panic(fmt.Sprintf("invalid foreground color for verb %q: %v", label, err)) + } mu.Lock() defer mu.Unlock() + if locked { + panic(fmt.Sprintf("failed to register verb style %q: style-related registrations must be done in task/inspection/**/contract packages during package initialization. Did you call it from outside of the contract or not at package initialization timing?", label)) + } + id := uint32(len(verbs) + 1) v := &pb.Verb{ Id: proto.Uint32(id), @@ -148,12 +207,22 @@ func RegisterVerb(label string, backgroundColor Color, foregroundColor Color, vi return v } -// RegisterLogType registers a LogType, assigns a unique ID to it, +// MustRegisterLogType registers a LogType, assigns a unique ID to it, // and returns the generated pointer. -func RegisterLogType(label string, description string, backgroundColor Color, foregroundColor Color) *pb.LogType { +func MustRegisterLogType(label string, description string, backgroundColor Color, foregroundColor Color) *pb.LogType { + if err := backgroundColor.Verify(); err != nil { + panic(fmt.Sprintf("invalid background color for log type %q: %v", label, err)) + } + if err := foregroundColor.Verify(); err != nil { + panic(fmt.Sprintf("invalid foreground color for log type %q: %v", label, err)) + } mu.Lock() defer mu.Unlock() + if locked { + panic(fmt.Sprintf("failed to register log type style %q: style-related registrations must be done in task/inspection/**/contract packages during package initialization. Did you call it from outside of the contract or not at package initialization timing?", label)) + } + id := uint32(len(logTypes) + 1) l := &pb.LogType{ Id: proto.Uint32(id), @@ -166,12 +235,19 @@ func RegisterLogType(label string, description string, backgroundColor Color, fo return l } -// RegisterRevisionState registers a RevisionState, assigns a unique ID to it, +// MustRegisterRevisionState registers a RevisionState, assigns a unique ID to it, // and returns the generated pointer. -func RegisterRevisionState(label string, icon string, description string, backgroundColor Color, style pb.RevisionStateStyle) *pb.RevisionState { +func MustRegisterRevisionState(label string, icon string, description string, backgroundColor Color, style pb.RevisionStateStyle) *pb.RevisionState { + if err := backgroundColor.Verify(); err != nil { + panic(fmt.Sprintf("invalid background color for revision state %q: %v", label, err)) + } mu.Lock() defer mu.Unlock() + if locked { + panic(fmt.Sprintf("failed to register revision state style %q: style-related registrations must be done in task/inspection/**/contract packages during package initialization. Did you call it from outside of the contract or not at package initialization timing?", label)) + } + id := uint32(len(revisionStates) + 1) rs := &pb.RevisionState{ Id: proto.Uint32(id), diff --git a/pkg/model/khifile/v6/style/timeline_test.go b/pkg/model/khifile/v6/style/timeline_test.go index e6afe05c6..4bfc518d6 100644 --- a/pkg/model/khifile/v6/style/timeline_test.go +++ b/pkg/model/khifile/v6/style/timeline_test.go @@ -15,6 +15,7 @@ package style import ( + "fmt" "sync" "testing" @@ -24,23 +25,23 @@ import ( func TestRegisterTimelineType(t *testing.T) { reset() - res1 := RegisterTimelineType("Type 1", "Desc 1", Color{1, 1, 1, 1}, Color{0, 0, 0, 1}, true, 1) - res2 := RegisterTimelineType("Type 2", "Desc 2", Color{1, 1, 1, 1}, Color{0, 0, 0, 1}, true, 2) + res1 := MustRegisterTimelineType("Type 1", "Desc 1", Color{1, 1, 1, 1}, Color{0, 0, 0, 1}, true, 1) + res2 := MustRegisterTimelineType("Type 2", "Desc 2", Color{1, 1, 1, 1}, Color{0, 0, 0, 1}, true, 2) // Verify IDs were assigned starting from 1 if res1.Id == nil || *res1.Id != 1 { - t.Errorf("Expected ID 1, got %v", res1.Id) + t.Errorf("expected ID 1, got %v", res1.Id) } if res2.Id == nil || *res2.Id != 2 { - t.Errorf("Expected ID 2, got %v", res2.Id) + t.Errorf("expected ID 2, got %v", res2.Id) } chunk := GenerateChunk() if len(chunk.TimelineTypes) != 2 { - t.Fatalf("Expected 2 TimelineTypes in chunk, got %d", len(chunk.TimelineTypes)) + t.Fatalf("expected 2 TimelineTypes in chunk, got %d", len(chunk.TimelineTypes)) } if *chunk.TimelineTypes[0].Id != 1 { - t.Errorf("Expected first chunk TimelineType ID to be 1") + t.Errorf("expected first chunk TimelineType ID to be 1") } } @@ -54,7 +55,7 @@ func TestRegisterConcurrent(t *testing.T) { for i := 0; i < numGoroutines; i++ { go func(index int) { defer wg.Done() - RegisterVerb("Concurrent Verb", Color{1, 1, 1, 1}, Color{0, 0, 0, 1}, true) + MustRegisterVerb("Concurrent Verb", Color{1, 1, 1, 1}, Color{0, 0, 0, 1}, true) }(i) } @@ -62,51 +63,51 @@ func TestRegisterConcurrent(t *testing.T) { chunk := GenerateChunk() if len(chunk.Verbs) != numGoroutines { - t.Fatalf("Expected %d Verbs registered, got %d", numGoroutines, len(chunk.Verbs)) + t.Fatalf("expected %d Verbs registered, got %d", numGoroutines, len(chunk.Verbs)) } // Verify all IDs from 1 to numGoroutines exist exactly once idMap := make(map[uint32]bool) for _, v := range chunk.Verbs { if v.Id == nil { - t.Fatalf("Verb ID is nil") + t.Fatalf("verb ID is nil") } if idMap[*v.Id] { - t.Errorf("Duplicate ID found: %d", *v.Id) + t.Errorf("duplicate ID found: %d", *v.Id) } idMap[*v.Id] = true } if len(idMap) != numGoroutines { - t.Errorf("Expected %d unique IDs, got %d", numGoroutines, len(idMap)) + t.Errorf("expected %d unique IDs, got %d", numGoroutines, len(idMap)) } } func TestGenerateChunkHasAllSlices(t *testing.T) { reset() - RegisterSeverity("Sev", "S", Color{1, 1, 1, 1}, Color{0, 0, 0, 1}, 1) - RegisterVerb("Verb", Color{1, 1, 1, 1}, Color{0, 0, 0, 1}, true) - RegisterLogType("Log", "Desc", Color{1, 1, 1, 1}, Color{0, 0, 0, 1}) - RegisterRevisionState("RevState", "icon", "Desc", Color{1, 1, 1, 1}, pb.RevisionStateStyle_REVISION_STATE_STYLE_NORMAL) - RegisterTimelineType("Timeline", "Desc", Color{1, 1, 1, 1}, Color{0, 0, 0, 1}, true, 1) + MustRegisterSeverity("Sev", "S", Color{1, 1, 1, 1}, Color{0, 0, 0, 1}, 1) + MustRegisterVerb("Verb", Color{1, 1, 1, 1}, Color{0, 0, 0, 1}, true) + MustRegisterLogType("Log", "Desc", Color{1, 1, 1, 1}, Color{0, 0, 0, 1}) + MustRegisterRevisionState("RevState", "icon", "Desc", Color{1, 1, 1, 1}, pb.RevisionStateStyle_REVISION_STATE_STYLE_NORMAL) + MustRegisterTimelineType("Timeline", "Desc", Color{1, 1, 1, 1}, Color{0, 0, 0, 1}, true, 1) chunk := GenerateChunk() if len(chunk.Severities) != 1 { - t.Errorf("Expected 1 Severity, got %d", len(chunk.Severities)) + t.Errorf("expected 1 Severity, got %d", len(chunk.Severities)) } if len(chunk.Verbs) != 1 { - t.Errorf("Expected 1 Verb, got %d", len(chunk.Verbs)) + t.Errorf("expected 1 Verb, got %d", len(chunk.Verbs)) } if len(chunk.LogTypes) != 1 { - t.Errorf("Expected 1 LogType, got %d", len(chunk.LogTypes)) + t.Errorf("expected 1 LogType, got %d", len(chunk.LogTypes)) } if len(chunk.RevisionStates) != 1 { - t.Errorf("Expected 1 RevisionState, got %d", len(chunk.RevisionStates)) + t.Errorf("expected 1 RevisionState, got %d", len(chunk.RevisionStates)) } if len(chunk.TimelineTypes) != 1 { - t.Errorf("Expected 1 TimelineType, got %d", len(chunk.TimelineTypes)) + t.Errorf("expected 1 TimelineType, got %d", len(chunk.TimelineTypes)) } } @@ -115,18 +116,157 @@ func TestGenerateChunkHasIconAtlas(t *testing.T) { chunk := GenerateChunk() if chunk.IconAtlas == nil { - t.Fatal("Expected IconAtlas not to be nil in TimelineStyleChunk") + t.Fatal("expected IconAtlas not to be nil in TimelineStyleChunk") } if len(chunk.IconAtlas.MsdfIconImage) == 0 || len(chunk.IconAtlas.MsdfIconImage[0]) == 0 { - t.Error("Expected non-empty embedded PNG bytes in MsdfIconImage") + t.Error("expected non-empty embedded PNG bytes in MsdfIconImage") } if len(chunk.IconAtlas.BmfontJson) == 0 { - t.Error("Expected non-empty embedded BMFont configuration JSON") + t.Error("expected non-empty embedded BMFont configuration JSON") } if len(chunk.IconAtlas.NameToCodepoints) == 0 { - t.Error("Expected populated mapping in NameToCodepoints") + t.Error("expected populated mapping in NameToCodepoints") + } +} + +func TestColorVerify(t *testing.T) { + testCases := []struct { + name string + color Color + wantErr bool + }{ + { + name: "valid color", + color: Color{R: 0.5, G: 0.5, B: 0.5, A: 1.0}, + wantErr: false, + }, + { + name: "invalid R (negative)", + color: Color{R: -0.1, G: 0.5, B: 0.5, A: 1.0}, + wantErr: true, + }, + { + name: "invalid R (too high)", + color: Color{R: 1.1, G: 0.5, B: 0.5, A: 1.0}, + wantErr: true, + }, + { + name: "invalid G (negative)", + color: Color{R: 0.5, G: -0.1, B: 0.5, A: 1.0}, + wantErr: true, + }, + { + name: "invalid G (too high)", + color: Color{R: 0.5, G: 1.1, B: 0.5, A: 1.0}, + wantErr: true, + }, + { + name: "invalid B (negative)", + color: Color{R: 0.5, G: 0.5, B: -0.1, A: 1.0}, + wantErr: true, + }, + { + name: "invalid B (too high)", + color: Color{R: 0.5, G: 0.5, B: 1.1, A: 1.0}, + wantErr: true, + }, + { + name: "invalid A (negative)", + color: Color{R: 0.5, G: 0.5, B: 0.5, A: -0.1}, + wantErr: true, + }, + { + name: "invalid A (too high)", + color: Color{R: 0.5, G: 0.5, B: 0.5, A: 1.1}, + wantErr: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := tc.color.Verify() + if (err != nil) != tc.wantErr { + t.Errorf("Color.Verify() error = %v, wantErr = %v", err, tc.wantErr) + } + }) + } +} + +func TestLockRegistry(t *testing.T) { + testCases := []struct { + name string + label string + styleClass string + fn func() + }{ + { + name: "RegisterTimelineType", + label: "T", + styleClass: "timeline type", + fn: func() { + MustRegisterTimelineType("T", "D", Color{1, 1, 1, 1}, Color{0, 0, 0, 1}, true, 1) + }, + }, + { + name: "RegisterSeverity", + label: "S", + styleClass: "severity", + fn: func() { + MustRegisterSeverity("S", "S", Color{1, 1, 1, 1}, Color{0, 0, 0, 1}, 1) + }, + }, + { + name: "RegisterVerb", + label: "V", + styleClass: "verb", + fn: func() { + MustRegisterVerb("V", Color{1, 1, 1, 1}, Color{0, 0, 0, 1}, true) + }, + }, + { + name: "RegisterLogType", + label: "L", + styleClass: "log type", + fn: func() { + MustRegisterLogType("L", "D", Color{1, 1, 1, 1}, Color{0, 0, 0, 1}) + }, + }, + { + name: "RegisterRevisionState", + label: "R", + styleClass: "revision state", + fn: func() { + MustRegisterRevisionState("R", "I", "D", Color{1, 1, 1, 1}, pb.RevisionStateStyle_REVISION_STATE_STYLE_NORMAL) + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + reset() + LockRegistry() + + defer func() { + r := recover() + if r == nil { + t.Errorf("expected panic, but did not panic") + return + } + msg, ok := r.(string) + if !ok { + t.Errorf("expected panic message to be a string, got %T", r) + return + } + wantMsg := fmt.Sprintf("failed to register %s style %q: style-related registrations must be done in task/inspection/**/contract packages during package initialization. Did you call it from outside of the contract or not at package initialization timing?", tc.styleClass, tc.label) + if msg != wantMsg { + t.Errorf("unexpected panic message: got %q, want %q", msg, wantMsg) + } + }() + + tc.fn() + }) } } diff --git a/pkg/task/inspection/commonlogk8saudit/contract/styles.go b/pkg/task/inspection/commonlogk8saudit/contract/styles.go index 3717dd080..f0233a658 100644 --- a/pkg/task/inspection/commonlogk8saudit/contract/styles.go +++ b/pkg/task/inspection/commonlogk8saudit/contract/styles.go @@ -20,7 +20,7 @@ import ( ) // RevisionStateK8sResourceExisting is the style for a resource that is existing. -var RevisionStateK8sResourceExisting = style.RegisterRevisionState( +var RevisionStateK8sResourceExisting = style.MustRegisterRevisionState( "Resource is existing", "deployed_code", "Resource is existing", @@ -29,7 +29,7 @@ var RevisionStateK8sResourceExisting = style.RegisterRevisionState( ) // RevisionStateK8sResourceDeleting is the style for a resource that is under deletion with graceful termination period or finalizer. -var RevisionStateK8sResourceDeleting = style.RegisterRevisionState( +var RevisionStateK8sResourceDeleting = style.MustRegisterRevisionState( "Resource is under deletion with graceful termination period or finalizer", "auto_delete", "Resource is under deletion with graceful termination period or finalizer", @@ -38,7 +38,7 @@ var RevisionStateK8sResourceDeleting = style.RegisterRevisionState( ) // RevisionStateK8sResourceIsDeleted is the style for a resource that is deleted. -var RevisionStateK8sResourceIsDeleted = style.RegisterRevisionState( +var RevisionStateK8sResourceIsDeleted = style.MustRegisterRevisionState( "Resource is deleted", "delete_forever", "Resource is deleted", diff --git a/pkg/testutil/inspection/conformance.go b/pkg/testutil/inspection/conformance.go index 266fa7793..664ebccc5 100644 --- a/pkg/testutil/inspection/conformance.go +++ b/pkg/testutil/inspection/conformance.go @@ -21,6 +21,7 @@ import ( coreinspection "github.com/GoogleCloudPlatform/khi/pkg/core/inspection" "github.com/GoogleCloudPlatform/khi/pkg/generated" + "github.com/GoogleCloudPlatform/khi/pkg/model/khifile/v6/style" inspectioncore_contract "github.com/GoogleCloudPlatform/khi/pkg/task/inspection/inspectioncore/contract" ) @@ -37,6 +38,7 @@ func ConformanceTestForInspectionTypes(t *testing.T) { if err != nil { t.Fatalf("unexpected error %v. failed to complete the preparation step", err) } + style.LockRegistry() for _, inspectionType := range testServer.GetAllInspectionTypes() { t.Run(fmt.Sprintf("%s-contains-at-least-one-feature", inspectionType.Name), func(t *testing.T) {