Skip to content

Commit 08822cd

Browse files
committed
review: address code review feedback on startup TUI and env_when
- Use errors.New instead of fmt.Errorf for sentinel with no wrapping - Remove no-op hintTextStyle, apply hint text directly - Document splitErrHint convention for startup check implementors - Document env/env_when priority in buildEnv - Add test for feature gate env_when validation - Add test for multi-level template env_when inheritance
1 parent 9b64622 commit 08822cd

5 files changed

Lines changed: 79 additions & 3 deletions

File tree

actions/runner.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,8 @@ func buildEnv(cfg *config.GitteConfig, st *state.GitteState, projName string, pr
357357
for k, v := range projEnv {
358358
envMap[k] = v
359359
}
360+
// env_when overrides env for the same key on matching arch; on non-matching
361+
// arch the key from env remains (or is absent if not in env).
360362
for k, v := range projEnvWhen {
361363
envMap[k] = v
362364
}

config/template_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,3 +372,40 @@ func TestResolveTemplates_EnvWhenMergeDisjointKeys(t *testing.T) {
372372
t.Error("expected FROM_PROJECT from project")
373373
}
374374
}
375+
376+
func TestResolveTemplates_EnvWhenMultiLevelInheritance(t *testing.T) {
377+
// grandparent → parent → project: env_when propagates through the chain.
378+
cfg := &GitteConfig{
379+
Templates: map[string]Template{
380+
"grandparent": {
381+
EnvWhen: map[string]EnvWhenEntry{
382+
"FROM_GRANDPARENT": {Value: "gp"},
383+
},
384+
},
385+
"parent": {
386+
Extends: []string{"grandparent"},
387+
EnvWhen: map[string]EnvWhenEntry{
388+
"FROM_PARENT": {Value: "p"},
389+
},
390+
},
391+
},
392+
Projects: map[string]ProjectConfig{
393+
"myservice": {
394+
Remote: "git@github.com:example/myservice.git",
395+
Extends: "parent",
396+
},
397+
},
398+
}
399+
400+
if err := ResolveTemplates(cfg); err != nil {
401+
t.Fatalf("unexpected error: %v", err)
402+
}
403+
404+
proj := cfg.Projects["myservice"]
405+
if proj.EnvWhen["FROM_GRANDPARENT"].Value != "gp" {
406+
t.Errorf("expected FROM_GRANDPARENT to propagate through parent, got %v", proj.EnvWhen)
407+
}
408+
if proj.EnvWhen["FROM_PARENT"].Value != "p" {
409+
t.Errorf("expected FROM_PARENT in resolved project, got %v", proj.EnvWhen)
410+
}
411+
}

config/validate_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,3 +143,38 @@ func TestValidateConfig_EnvWhenValidArchType(t *testing.T) {
143143
t.Errorf("expected no errors, got: %v", result.Errors)
144144
}
145145
}
146+
147+
func TestValidateConfig_EnvWhenFeatureGateUnknownConditionType(t *testing.T) {
148+
cfg := &GitteConfig{
149+
FeatureGates: map[string]FeatureGate{
150+
"my-feature": {
151+
Effects: FeatureEffects{
152+
EnvWhen: map[string]EnvWhenEntry{
153+
"FOO": {
154+
Value: "bar",
155+
Conditions: []EnvWhenCondition{
156+
{Type: "not_a_real_type"},
157+
},
158+
},
159+
},
160+
},
161+
},
162+
},
163+
}
164+
165+
result := ValidateConfig(cfg)
166+
if !result.HasErrors() {
167+
t.Fatal("expected validation error for unknown condition type in feature gate")
168+
}
169+
170+
found := false
171+
for _, e := range result.Errors {
172+
if strings.Contains(e.Field, "env_when") && strings.Contains(e.Message, "not_a_real_type") {
173+
found = true
174+
break
175+
}
176+
}
177+
if !found {
178+
t.Errorf("expected error mentioning env_when and 'not_a_real_type', got: %v", result.Errors)
179+
}
180+
}

startup/startup.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package startup
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"sort"
78

@@ -56,7 +57,7 @@ func Run(ctx context.Context, cfg *config.GitteConfig, cwd string, mode output.O
5657
if runErr != nil && mode != output.ModePlain {
5758
// TUI view already printed a human-readable failure summary; return a
5859
// terse sentinel so root.go only prints "startup checks failed".
59-
return fmt.Errorf("startup checks failed")
60+
return errors.New("startup checks failed")
6061
}
6162
return runErr
6263
}

startup/view.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ func (v *tuiView) printFailureSummary() {
158158
fmt.Printf(" %s %s %s\n", failStyle.Render("✗"), failStyle.Render(f.name), dimStyle.Render(fmtDuration(f.elapsed)))
159159
fmt.Printf(" %s\n", dimStyle.Render(f.errMsg))
160160
if f.hint != "" {
161-
fmt.Printf(" %s %s\n", hintLabelStyle.Render("hint:"), hintTextStyle.Render(f.hint))
161+
fmt.Printf(" %s %s\n", hintLabelStyle.Render("hint:"), f.hint)
162162
}
163163
fmt.Println()
164164
}
@@ -175,7 +175,6 @@ var (
175175
titleStyle = lipgloss.NewStyle().Bold(true).Foreground(lipgloss.Color("170"))
176176
dimStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("241"))
177177
hintLabelStyle = lipgloss.NewStyle().Bold(true).Foreground(lipgloss.Color("214"))
178-
hintTextStyle = lipgloss.NewStyle()
179178
)
180179

181180
var spinnerFrames = []string{"⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"}
@@ -397,6 +396,8 @@ func truncateToVisualWidth(s string, maxWidth int) string {
397396

398397
// splitErrHint splits an error message of the form "msg\nhint: hint" into its
399398
// two parts. If there is no hint suffix the second return value is empty.
399+
// Startup checks emit hints by appending "\nhint: <text>" to their error
400+
// message (see startup.go where checks call check.GetHint()).
400401
func splitErrHint(err error) (string, string) {
401402
const sep = "\nhint: "
402403
msg := err.Error()

0 commit comments

Comments
 (0)