diff --git a/pkg/vars/variables.go b/pkg/vars/variables.go index c8f785c9..6d8eaf3e 100644 --- a/pkg/vars/variables.go +++ b/pkg/vars/variables.go @@ -262,6 +262,12 @@ func (v *Variables) ResolveQuery(queryStr string) (val interface{}, ok bool, err return nil, false, nil } + // gojq reports runtime evaluation errors as values of type error, so a + // failed query must be surfaced instead of being returned as a result. + if queryErr, isErr := val.(error); isErr { + return nil, false, fmt.Errorf("error evaluating variable query '%v': %w", queryStr, queryErr) + } + return val, true, nil } @@ -296,6 +302,12 @@ func (v *Variables) ConsumeVars(config interface{}, consumeMap map[string]string continue } + // gojq reports runtime evaluation errors as values of type error, so a + // failed query must be surfaced instead of being applied to the config. + if queryErr, isErr := val.(error); isErr { + return fmt.Errorf("error evaluating variable query '%v': %w", queryStr, queryErr) + } + if v, ok := val.(float64); ok { val = NoScientificFloat64(v) } diff --git a/pkg/vars/variables_jq_test.go b/pkg/vars/variables_jq_test.go new file mode 100644 index 00000000..c84e9ceb --- /dev/null +++ b/pkg/vars/variables_jq_test.go @@ -0,0 +1,76 @@ +package vars_test + +import ( + "testing" + + "github.com/ethpandaops/assertoor/pkg/vars" +) + +// TestResolveQuerySurfacesRuntimeError verifies that a gojq runtime error is returned +// as an error rather than as a successful value. gojq yields evaluation errors as +// values of type error with ok=true, so without an explicit check an erroring query +// (here, adding a number to a string) was reported as a result. In the task `if:` +// path that meant the condition was silently treated as non-boolean and the task was +// skipped instead of failing. +func TestResolveQuerySurfacesRuntimeError(t *testing.T) { + scope := vars.NewVariables(nil) + scope.SetVar("n", "hello") + + val, ok, err := scope.ResolveQuery(".n + 1") + if err == nil { + t.Fatalf("expected an error for an erroring query, got value %v (ok=%v)", val, ok) + } + + if _, isErr := val.(error); isErr { + t.Fatal("the gojq error value leaked out as the result value") + } +} + +// TestResolveQueryValidQuery guards the happy path so the error check does not reject +// good queries. +func TestResolveQueryValidQuery(t *testing.T) { + scope := vars.NewVariables(nil) + scope.SetVar("greeting", "hello") + + val, ok, err := scope.ResolveQuery(".greeting") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if !ok || val != "hello" { + t.Fatalf("got (%v, ok=%v), want (\"hello\", true)", val, ok) + } +} + +// TestConsumeVarsSurfacesRuntimeError verifies that an erroring configVars query is +// surfaced as an error rather than writing the error object into the config. +func TestConsumeVarsSurfacesRuntimeError(t *testing.T) { + scope := vars.NewVariables(nil) + scope.SetVar("n", "hello") + + var cfg struct { + X int `yaml:"x"` + } + + if err := scope.ConsumeVars(&cfg, map[string]string{"x": ".n + 1"}); err == nil { + t.Fatal("expected an error for an erroring configVars query, got nil") + } +} + +// TestConsumeVarsValidQuery guards the happy path. +func TestConsumeVarsValidQuery(t *testing.T) { + scope := vars.NewVariables(nil) + scope.SetVar("num", 5) + + var cfg struct { + X int `yaml:"x"` + } + + if err := scope.ConsumeVars(&cfg, map[string]string{"x": ".num"}); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if cfg.X != 5 { + t.Fatalf("cfg.X = %d, want 5", cfg.X) + } +}