From 9ef5f58f885eb35d0759f73cf0d5caba73ea3dd2 Mon Sep 17 00:00:00 2001 From: mmetc <92726601+mmetc@users.noreply.github.com> Date: Wed, 15 Jan 2025 12:13:54 +0100 Subject: [PATCH] test pkg/exprhelpers: explicit message if the tag "expr_debug" is missing (#3400) * test pkg/exprhelpers: explicit message if the tag "expr_debug" is missing * typo * lint: use build tag expr_debug while linting * lint --- .github/workflows/go-tests.yml | 4 +- .golangci.yml | 4 + pkg/exprhelpers/debugger.go | 112 +++++++++++++++++++-------- pkg/exprhelpers/debugger_test.go | 1 + pkg/exprhelpers/debuggerstub_test.go | 10 +++ 5 files changed, 95 insertions(+), 36 deletions(-) create mode 100644 pkg/exprhelpers/debuggerstub_test.go diff --git a/.github/workflows/go-tests.yml b/.github/workflows/go-tests.yml index 3a194e1084a..d882f88580e 100644 --- a/.github/workflows/go-tests.yml +++ b/.github/workflows/go-tests.yml @@ -143,11 +143,11 @@ jobs: go generate ./... protoc --version if [[ $(git status --porcelain) ]]; then - echo "Error: Uncommitted changes found after running 'make generate'. Please commit all generated code." + echo "Error: Uncommitted changes found after running 'go generate'. Please commit all generated code." git diff exit 1 else - echo "No changes detected after running 'make generate'." + echo "No changes detected after running 'go generate'." fi - name: Create localstack streams diff --git a/.golangci.yml b/.golangci.yml index deb073c2eea..5995f14c512 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,5 +1,9 @@ # https://github.com/golangci/golangci-lint/blob/master/.golangci.reference.yml +run: + build-tags: + - expr_debug + linters-settings: gci: sections: diff --git a/pkg/exprhelpers/debugger.go b/pkg/exprhelpers/debugger.go index 2e47af6d1de..d44b8fc97e1 100644 --- a/pkg/exprhelpers/debugger.go +++ b/pkg/exprhelpers/debugger.go @@ -21,35 +21,35 @@ var IndentStep = 4 // we use this struct to store the output of the expr runtime type OpOutput struct { - Code string //relevant code part + Code string // relevant code part - CodeDepth int //level of nesting + CodeDepth int // level of nesting BlockStart bool BlockEnd bool - Func bool //true if it's a function call + Func bool // true if it's a function call FuncName string Args []string FuncResults []string // - Comparison bool //true if it's a comparison + Comparison bool // true if it's a comparison Negated bool Left string Right string // - JumpIf bool //true if it's conditional jump + JumpIf bool // true if it's conditional jump IfTrue bool IfFalse bool // - Condition bool //true if it's a condition + Condition bool // true if it's a condition ConditionIn bool ConditionContains bool - //used for comparisons, conditional jumps and conditions + // used for comparisons, conditional jumps and conditions StrConditionResult string - ConditionResult *bool //should always be present for conditions + ConditionResult *bool // should always be present for conditions // - Finalized bool //used when a node is finalized, we already fetched result from next OP + Finalized bool // used when a node is finalized, we already fetched result from next OP } func (o *OpOutput) String() string { @@ -57,6 +57,7 @@ func (o *OpOutput) String() string { if o.Code != "" { ret += fmt.Sprintf("[%s]", o.Code) } + ret += " " switch { @@ -68,19 +69,24 @@ func (o *OpOutput) String() string { if indent < 0 { indent = 0 } + ret = fmt.Sprintf("%*cBLOCK_END [%s]", indent, ' ', o.Code) + if o.StrConditionResult != "" { ret += fmt.Sprintf(" -> %s", o.StrConditionResult) } + return ret - //A block end can carry a value, for example if it's a count, any, all etc. XXX + // A block end can carry a value, for example if it's a count, any, all etc. XXX case o.Func: return ret + fmt.Sprintf("%s(%s) = %s", o.FuncName, strings.Join(o.Args, ", "), strings.Join(o.FuncResults, ", ")) case o.Comparison: if o.Negated { ret += "NOT " } + ret += fmt.Sprintf("%s == %s -> %s", o.Left, o.Right, o.StrConditionResult) + return ret case o.ConditionIn: return ret + fmt.Sprintf("%s in %s -> %s", o.Args[0], o.Args[1], o.StrConditionResult) @@ -91,18 +97,23 @@ func (o *OpOutput) String() string { if *o.ConditionResult { return ret + "OR -> false" } + return ret + "OR -> true" } + return ret + "OR(?)" case o.JumpIf && o.IfFalse: if o.ConditionResult != nil { if *o.ConditionResult { return ret + "AND -> true" } + return ret + "AND -> false" } + return ret + "AND(?)" } + return ret + "" } @@ -135,7 +146,7 @@ func (erp ExprRuntimeDebug) extractCode(ip int, program *vm.Program) string { func autoQuote(v any) string { switch x := v.(type) { case string: - //let's avoid printing long strings. it can happen ie. when we are debugging expr with `File()` or similar helpers + // let's avoid printing long strings. it can happen ie. when we are debugging expr with `File()` or similar helpers if len(x) > 40 { return fmt.Sprintf("%q", x[:40]+"...") } else { @@ -147,35 +158,40 @@ func autoQuote(v any) string { } func (erp ExprRuntimeDebug) ipDebug(ip int, vm *vm.VM, program *vm.Program, parts []string, outputs []OpOutput) ([]OpOutput, error) { - IdxOut := len(outputs) prevIdxOut := 0 currentDepth := 0 - //when there is a function call or comparison, we need to wait for the next instruction to get the result and "finalize" the previous one + // when there is a function call or comparison, we need to wait for the next instruction to get the result and "finalize" the previous one if IdxOut > 0 { prevIdxOut = IdxOut - 1 currentDepth = outputs[prevIdxOut].CodeDepth + if outputs[prevIdxOut].Func && !outputs[prevIdxOut].Finalized { stack := vm.Stack num_items := 1 + for i := len(stack) - 1; i >= 0 && num_items > 0; i-- { outputs[prevIdxOut].FuncResults = append(outputs[prevIdxOut].FuncResults, autoQuote(stack[i])) num_items-- } + outputs[prevIdxOut].Finalized = true } else if (outputs[prevIdxOut].Comparison || outputs[prevIdxOut].Condition) && !outputs[prevIdxOut].Finalized { stack := vm.Stack outputs[prevIdxOut].StrConditionResult = fmt.Sprintf("%+v", stack) + if val, ok := stack[0].(bool); ok { outputs[prevIdxOut].ConditionResult = new(bool) *outputs[prevIdxOut].ConditionResult = val } + outputs[prevIdxOut].Finalized = true } } erp.Logger.Tracef("[STEP %d:%s] (stack:%+v) (parts:%+v) {depth:%d}", ip, parts[1], vm.Stack, parts, currentDepth) + out := OpOutput{} out.CodeDepth = currentDepth out.Code = erp.extractCode(ip, program) @@ -188,27 +204,28 @@ func (erp ExprRuntimeDebug) ipDebug(ip int, vm *vm.VM, program *vm.Program, part case "OpEnd": out.CodeDepth -= IndentStep out.BlockEnd = true - //OpEnd can carry value, if it's any/all/count etc. + // OpEnd can carry value, if it's any/all/count etc. if len(vm.Stack) > 0 { out.StrConditionResult = fmt.Sprintf("%v", vm.Stack) } + outputs = append(outputs, out) case "OpNot": - //negate the previous condition + // negate the previous condition outputs[prevIdxOut].Negated = true - case "OpTrue": //generated when possible ? (1 == 1) + case "OpTrue": // generated when possible ? (1 == 1) out.Condition = true out.ConditionResult = new(bool) *out.ConditionResult = true out.StrConditionResult = "true" outputs = append(outputs, out) - case "OpFalse": //generated when possible ? (1 != 1) + case "OpFalse": // generated when possible ? (1 != 1) out.Condition = true out.ConditionResult = new(bool) *out.ConditionResult = false out.StrConditionResult = "false" outputs = append(outputs, out) - case "OpJumpIfTrue": //OR + case "OpJumpIfTrue": // OR stack := vm.Stack out.JumpIf = true out.IfTrue = true @@ -218,78 +235,88 @@ func (erp ExprRuntimeDebug) ipDebug(ip int, vm *vm.VM, program *vm.Program, part out.ConditionResult = new(bool) *out.ConditionResult = val } + outputs = append(outputs, out) - case "OpJumpIfFalse": //AND + case "OpJumpIfFalse": // AND stack := vm.Stack out.JumpIf = true out.IfFalse = true out.StrConditionResult = fmt.Sprintf("%v", stack[0]) + if val, ok := stack[0].(bool); ok { out.ConditionResult = new(bool) *out.ConditionResult = val } + outputs = append(outputs, out) - case "OpCall1": //Op for function calls + case "OpCall1": // Op for function calls out.Func = true out.FuncName = parts[3] stack := vm.Stack + num_items := 1 for i := len(stack) - 1; i >= 0 && num_items > 0; i-- { out.Args = append(out.Args, autoQuote(stack[i])) num_items-- } + outputs = append(outputs, out) - case "OpCall2": //Op for function calls + case "OpCall2": // Op for function calls out.Func = true out.FuncName = parts[3] stack := vm.Stack + num_items := 2 for i := len(stack) - 1; i >= 0 && num_items > 0; i-- { out.Args = append(out.Args, autoQuote(stack[i])) num_items-- } + outputs = append(outputs, out) - case "OpCall3": //Op for function calls + case "OpCall3": // Op for function calls out.Func = true out.FuncName = parts[3] stack := vm.Stack + num_items := 3 for i := len(stack) - 1; i >= 0 && num_items > 0; i-- { out.Args = append(out.Args, autoQuote(stack[i])) num_items-- } + outputs = append(outputs, out) - //double check OpCallFast and OpCallTyped + // double check OpCallFast and OpCallTyped case "OpCallFast", "OpCallTyped": // - case "OpCallN": //Op for function calls with more than 3 args + case "OpCallN": // Op for function calls with more than 3 args out.Func = true out.FuncName = parts[1] stack := vm.Stack - //for OpCallN, we get the number of args + // for OpCallN, we get the number of args if len(program.Arguments) >= ip { nb_args := program.Arguments[ip] if nb_args > 0 { - //we need to skip the top item on stack + // we need to skip the top item on stack for i := len(stack) - 2; i >= 0 && nb_args > 0; i-- { out.Args = append(out.Args, autoQuote(stack[i])) nb_args-- } } - } else { //let's blindly take the items on stack + } else { // let's blindly take the items on stack for _, val := range vm.Stack { out.Args = append(out.Args, autoQuote(val)) } } + outputs = append(outputs, out) - case "OpEqualString", "OpEqual", "OpEqualInt": //comparisons + case "OpEqualString", "OpEqual", "OpEqualInt": // comparisons stack := vm.Stack out.Comparison = true out.Left = autoQuote(stack[0]) out.Right = autoQuote(stack[1]) outputs = append(outputs, out) - case "OpIn": //in operator + case "OpIn": // in operator stack := vm.Stack out.Condition = true out.ConditionIn = true @@ -299,7 +326,7 @@ func (erp ExprRuntimeDebug) ipDebug(ip int, vm *vm.VM, program *vm.Program, part out.Args = append(out.Args, autoQuote(stack[0])) out.Args = append(out.Args, autoQuote(stack[1])) outputs = append(outputs, out) - case "OpContains": //kind OpIn , but reverse + case "OpContains": // kind OpIn , but reverse stack := vm.Stack out.Condition = true out.ConditionContains = true @@ -310,6 +337,7 @@ func (erp ExprRuntimeDebug) ipDebug(ip int, vm *vm.VM, program *vm.Program, part out.Args = append(out.Args, autoQuote(stack[1])) outputs = append(outputs, out) } + return outputs, nil } @@ -319,10 +347,12 @@ func (erp ExprRuntimeDebug) ipSeek(ip int) []string { if len(parts) == 0 { continue } + if parts[0] == strconv.Itoa(ip) { return parts } } + return nil } @@ -330,19 +360,23 @@ func Run(program *vm.Program, env interface{}, logger *log.Entry, debug bool) (a if debug { dbgInfo, ret, err := RunWithDebug(program, env, logger) DisplayExprDebug(program, dbgInfo, logger, ret) + return ret, err } + return expr.Run(program, env) } func cleanTextForDebug(text string) string { text = strings.Join(strings.Fields(text), " ") text = strings.Trim(text, " \t\n") + return text } func DisplayExprDebug(program *vm.Program, outputs []OpOutput, logger *log.Entry, ret any) { logger.Debugf("dbg(result=%v): %s", ret, cleanTextForDebug(string(program.Source()))) + for _, output := range outputs { logger.Debugf("%s", output.String()) } @@ -360,46 +394,55 @@ func RunWithDebug(program *vm.Program, env interface{}, logger *log.Entry) ([]Op erp.Lines = lines go func() { - //We must never return until the execution of the program is done + // We must never return until the execution of the program is done var err error + erp.Logger.Tracef("[START] ip 0") + ops := erp.ipSeek(0) if ops == nil { log.Warningf("error while debugging expr: failed getting ops for ip 0") } + if outputs, err = erp.ipDebug(0, vm, program, ops, outputs); err != nil { log.Warningf("error while debugging expr: error while debugging at ip 0") } + vm.Step() + for ip := range vm.Position() { ops := erp.ipSeek(ip) if ops == nil { erp.Logger.Tracef("[DONE] ip %d", ip) break } + if outputs, err = erp.ipDebug(ip, vm, program, ops, outputs); err != nil { log.Warningf("error while debugging expr: error while debugging at ip %d", ip) } + vm.Step() } }() var return_error error + ret, err := vm.Run(program, env) - //if the expr runtime failed, we don't need to wait for the debug to finish + // if the expr runtime failed, we don't need to wait for the debug to finish if err != nil { return_error = err } - //the overall result of expression is the result of last op ? + // the overall result of expression is the result of last op ? if len(outputs) > 0 { lastOutIdx := len(outputs) if lastOutIdx > 0 { lastOutIdx -= 1 } + switch val := ret.(type) { case bool: log.Tracef("completing with bool %t", ret) - //if outputs[lastOutIdx].Comparison { + // if outputs[lastOutIdx].Comparison { outputs[lastOutIdx].StrConditionResult = fmt.Sprintf("%v", ret) outputs[lastOutIdx].ConditionResult = new(bool) *outputs[lastOutIdx].ConditionResult = val @@ -412,5 +455,6 @@ func RunWithDebug(program *vm.Program, env interface{}, logger *log.Entry) ([]Op } else { log.Tracef("no output from expr runtime") } + return outputs, ret, return_error } diff --git a/pkg/exprhelpers/debugger_test.go b/pkg/exprhelpers/debugger_test.go index 32144454084..0852d7ab2de 100644 --- a/pkg/exprhelpers/debugger_test.go +++ b/pkg/exprhelpers/debugger_test.go @@ -1,3 +1,4 @@ +//go:build expr_debug package exprhelpers import ( diff --git a/pkg/exprhelpers/debuggerstub_test.go b/pkg/exprhelpers/debuggerstub_test.go new file mode 100644 index 00000000000..cc41c793b47 --- /dev/null +++ b/pkg/exprhelpers/debuggerstub_test.go @@ -0,0 +1,10 @@ +//go:build !expr_debug +package exprhelpers + +import ( + "testing" +) + +func TestFailWithoutExprDebug(t *testing.T) { + t.Fatal("To test pkg/exprhelpers, you need the expr_debug build tag") +}