Skip to content

Commit 694284b

Browse files
committed
fix: address PR feedback and clean up testdata files
- Remove manual feature gate additions from testdata files (auto-generated) - Fix unit test expectations to not expect markers in clean testdata - Update documentation to clarify testdata file handling - Ensure tests properly validate clean vs. modified testdata scenarios
1 parent 82c75d9 commit 694284b

File tree

6 files changed

+29
-50
lines changed

6 files changed

+29
-50
lines changed

docs/book/src/reference/markers/feature-gates.md

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -176,10 +176,10 @@ Bar *string `json:"bar,omitempty"`
176176

177177
2. **Beta**: Feature available but can be disabled
178178
```go
179-
// +feature-gate beta-feature
179+
// +feature-gate beta-feature
180180
BetaField *string `json:"betaField,omitempty"`
181181
```
182-
182+
183183
3. **Stable**: Remove feature gate, feature always available
184184
```go
185185
// No feature gate marker - always available
@@ -194,7 +194,7 @@ Test your controller with different feature gate configurations:
194194
# Test with all features disabled (default)
195195
make test
196196

197-
# Test with specific features enabled
197+
# Test with specific features enabled
198198
FEATURE_GATES="experimental-bar=true" make test
199199

200200
# Test with all features enabled
@@ -225,7 +225,7 @@ spec:
225225
226226
```yaml
227227
# Production - only stable features
228-
apiVersion: apps/v1
228+
apiVersion: apps/v1
229229
kind: Deployment
230230
metadata:
231231
name: controller-manager
@@ -244,7 +244,7 @@ spec:
244244
```yaml
245245
# Canary deployment with experimental features
246246
apiVersion: apps/v1
247-
kind: Deployment
247+
kind: Deployment
248248
metadata:
249249
name: controller-manager-canary
250250
spec:
@@ -316,7 +316,7 @@ grep -A 5 -B 5 "feature-gates" cmd/main.go
316316
### ✅ Production Ready
317317

318318
- Feature gate discovery from API type markers
319-
- CLI flag `--feature-gates` for runtime control
319+
- CLI flag `--feature-gates` for runtime control
320320
- Automatic scaffolding integration
321321
- Validation and error handling
322322
- Controller runtime integration
@@ -390,7 +390,7 @@ func (r *WebAppReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
390390
}
391391
}
392392

393-
// Handle advanced routing if enabled
393+
// Handle advanced routing if enabled
394394
if featureGates.IsEnabled("advanced-routing") && webapp.Spec.AdvancedRouting != nil {
395395
log.Info("Configuring advanced routing")
396396
if err := r.reconcileAdvancedRouting(ctx, &webapp); err != nil {

go.mod

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@ require (
1111
github.com/spf13/cobra v1.9.1
1212
github.com/spf13/pflag v1.0.7
1313
github.com/stretchr/testify v1.10.0
14-
golang.org/x/mod v0.26.0
15-
golang.org/x/text v0.27.0
16-
golang.org/x/tools v0.35.0
14+
golang.org/x/mod v0.27.0
15+
golang.org/x/text v0.28.0
16+
golang.org/x/tools v0.36.0
1717
helm.sh/helm/v3 v3.18.4
1818
sigs.k8s.io/yaml v1.6.0
1919
)

pkg/machinery/featuregate.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package machinery
1919
import (
2020
"fmt"
2121
"regexp"
22+
"sort"
2223
"strings"
2324
)
2425

@@ -63,8 +64,7 @@ func ParseFeatureGates(featureGates string) (FeatureGates, error) {
6364
for _, part := range parts {
6465
part = strings.TrimSpace(part)
6566
if part == "" {
66-
// Skip empty parts (e.g., trailing commas)
67-
continue
67+
return nil, fmt.Errorf("empty feature gate name in input")
6868
}
6969

7070
// Parse the feature gate name and value
@@ -141,8 +141,16 @@ func (fg FeatureGates) String() string {
141141
return ""
142142
}
143143

144+
// Get feature gate names and sort them for consistent output
145+
names := make([]string, 0, len(fg))
146+
for name := range fg {
147+
names = append(names, name)
148+
}
149+
sort.Strings(names)
150+
144151
var parts []string
145-
for name, enabled := range fg {
152+
for _, name := range names {
153+
enabled := fg[name]
146154
if enabled {
147155
parts = append(parts, name)
148156
} else {

pkg/plugins/golang/v4/scaffolds/api_test.go

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -186,22 +186,10 @@ func TestAPIScaffolder_discoverFeatureGates_Testdata(t *testing.T) {
186186

187187
featureGates := apiScaffolder.discoverFeatureGates()
188188

189-
// The testdata contains a feature gate marker for "experimental-bar"
190-
expectedGates := []string{"experimental-bar"}
189+
// The testdata should not contain any feature gate markers by default
190+
// since testdata files are auto-generated
191+
expectedGates := []string{}
191192
if len(featureGates) != len(expectedGates) {
192193
t.Errorf("Expected %d feature gates from testdata, but found %d: %v", len(expectedGates), len(featureGates), featureGates)
193194
}
194-
195-
for _, expectedGate := range expectedGates {
196-
found := false
197-
for _, gate := range featureGates {
198-
if gate == expectedGate {
199-
found = true
200-
break
201-
}
202-
}
203-
if !found {
204-
t.Errorf("Expected to find feature gate %s, but it was not discovered", expectedGate)
205-
}
206-
}
207195
}

test/e2e/v4/featuregates_discovery_test.go

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -45,27 +45,15 @@ var _ = Describe("Feature Gates Discovery", func() {
4545
})
4646

4747
Context("when parsing testdata project", func() {
48-
It("should discover feature gate markers from captain_types.go", func() {
48+
It("should NOT discover feature gate markers from captain_types.go by default", func() {
4949
By("parsing the testdata project API types")
5050
parser := machinery.NewFeatureGateMarkerParser()
5151
markers, err := parser.ParseFile("../../../testdata/project-v4/api/v1/captain_types.go")
5252
Expect(err).NotTo(HaveOccurred())
53-
Expect(markers).NotTo(BeEmpty(), "Should discover feature gate markers")
54-
55-
By("extracting feature gate names")
56-
featureGates := machinery.ExtractFeatureGates(markers)
57-
58-
By("verifying expected feature gates are found")
59-
expectedGates := []string{
60-
"experimental-bar",
61-
}
62-
63-
for _, expectedGate := range expectedGates {
64-
Expect(featureGates).To(ContainElement(expectedGate),
65-
"Should discover feature gate: %s", expectedGate)
66-
}
67-
68-
GinkgoWriter.Printf("Discovered feature gates: %v\n", featureGates)
53+
54+
By("verifying no feature gates are found in clean testdata")
55+
// Testdata files are auto-generated and should not contain feature gate markers by default
56+
Expect(markers).To(BeEmpty(), "Clean testdata should not contain feature gate markers")
6957
})
7058
})
7159

testdata/project-v4/api/v1/captain_types.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,6 @@ type CaptainSpec struct {
3333
// foo is an example field of Captain. Edit captain_types.go to remove/update
3434
// +optional
3535
Foo *string `json:"foo,omitempty"`
36-
37-
// Bar is an experimental field that requires the "experimental-bar" feature gate to be enabled
38-
// +feature-gate experimental-bar
39-
// +optional
40-
Bar *string `json:"bar,omitempty"`
4136
}
4237

4338
// CaptainStatus defines the observed state of Captain.

0 commit comments

Comments
 (0)