From 02338547792f29926d008a99b96956e2b6ad5f53 Mon Sep 17 00:00:00 2001 From: Yvonnick Esnault Date: Tue, 9 Feb 2021 17:46:00 +0100 Subject: [PATCH] fix(venom): use vars as string or struct (#361) * fix(venom): use vars as string or struct * fix(venom): display line number for info * add assertion example Signed-off-by: Yvonnick Esnault --- README.md | 8 ++++++++ assertion.go | 18 +++++++++++++---- assertions/assertions.go | 26 ++++++++++++++++++------ assertions/assertions_test.go | 7 +++++++ go.sum | 1 + process_testcase.go | 16 ++++++++++++++- process_teststep.go | 4 ++-- tests/lib/hello.yml | 3 ++- tests/play-with-vars.yml | 37 +++++++++++++++++++++++++++++++++++ tests/user_executor.yml | 13 +++++++++--- types.go | 2 +- types_executor.go | 10 +++++++++- 12 files changed, 126 insertions(+), 19 deletions(-) create mode 100644 tests/play-with-vars.yml diff --git a/README.md b/README.md index 0169459c..aa4b959e 100644 --- a/README.md +++ b/README.md @@ -215,6 +215,7 @@ vars: foo: foo biz: bar: bar + aString: '{"foo": "bar"}' testcases: - name: first-test-case @@ -224,6 +225,13 @@ testcases: assertions: - result.code ShouldEqual 0 - result.systemout ShouldEqual "foo bar" + +- name: foobar + steps: + - script: echo '{{.aString}}' + info: value of aString is {{.aString}} + assertions: + - result.systemoutjson.foo ShouldEqual bar ... ``` diff --git a/assertion.go b/assertion.go index 53b1149a..59ea75fb 100644 --- a/assertion.go +++ b/assertion.go @@ -190,7 +190,7 @@ func stringToType(val string, valType interface{}) (interface{}, error) { return val, nil } -func findLineNumber(filename, testcase string, stepNumber int, assertion string) int { +func findLineNumber(filename, testcase string, stepNumber int, assertion string, infoNumber int) int { countLine := 0 file, err := os.Open(filename) if err != nil { @@ -202,6 +202,7 @@ func findLineNumber(filename, testcase string, stepNumber int, assertion string) testcaseFound := false commentBlock := false countStep := 0 + countInfo := 0 scanner := bufio.NewScanner(file) for scanner.Scan() { @@ -226,9 +227,18 @@ func findLineNumber(filename, testcase string, stepNumber int, assertion string) countStep++ continue } - if testcaseFound && countStep > stepNumber && strings.Contains(line, assertion) { - lineFound = true - break + + if testcaseFound && countStep > stepNumber { + if strings.Contains(line, assertion) { + lineFound = true + break + } else if strings.Contains(strings.ReplaceAll(line, " ", ""), "info:") { + countInfo++ + if infoNumber == countInfo { + lineFound = true + break + } + } } } diff --git a/assertions/assertions.go b/assertions/assertions.go index 89c5510a..bbf8c73d 100644 --- a/assertions/assertions.go +++ b/assertions/assertions.go @@ -83,6 +83,23 @@ func deepEqual(x, y interface{}) bool { // - result.code ShouldEqual 0 // func ShouldEqual(actual interface{}, expected ...interface{}) error { + // if expected is an array, we considere that this array is an array of string + // so, we concat all values before doing the comparaison + if len(expected) > 0 { + var args string + for i := range expected { + actualS, err := cast.ToStringE(expected[i]) + if err != nil { + return err + } + args += actualS + " " + } + if deepEqual(actual, strings.TrimRight(args, " ")) { + return nil + } + return fmt.Errorf("expected: %v got: %v", args, actual) + } + if err := need(1, expected); err != nil { return err } @@ -94,13 +111,10 @@ func ShouldEqual(actual interface{}, expected ...interface{}) error { // ShouldNotEqual receives exactly two parameters and does an inequality check. func ShouldNotEqual(actual interface{}, expected ...interface{}) error { - if err := need(1, expected); err != nil { - return err + if err := ShouldEqual(actual, expected...); err == nil { + return fmt.Errorf("not expected: %v got: %v", expected[0], actual) } - if !deepEqual(actual, expected[0]) { - return nil - } - return fmt.Errorf("not expected: %v got: %v", expected[0], actual) + return nil } // ShouldAlmostEqual makes sure that two parameters are close enough to being equal. diff --git a/assertions/assertions_test.go b/assertions/assertions_test.go index f0f8d2d3..3ceb9c01 100644 --- a/assertions/assertions_test.go +++ b/assertions/assertions_test.go @@ -22,6 +22,13 @@ func TestShouldEqual(t *testing.T) { expected: []interface{}{`a`}, }, }, + { + name: "with string and multiple value", + args: args{ + actual: `foo bar goo`, + expected: []interface{}{`foo`, `bar`, `goo`}, + }, + }, { name: "with int", args: args{ diff --git a/go.sum b/go.sum index c2ee5c10..59dc93cf 100644 --- a/go.sum +++ b/go.sum @@ -399,6 +399,7 @@ github.com/json-iterator/go v1.1.6/go.mod h1:+SdeFBvtyEkXs7REEP0seUULqWtbJapLOCV github.com/json-iterator/go v1.1.7/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/uOdHXbAo4= github.com/json-iterator/go v1.1.8/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/uOdHXbAo4= github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024/go.mod h1:6v2b51hI/fHJwM22ozAgKL4VKDeJcHhJFhtBdhmNjmU= +github.com/jstemmer/go-junit-report v0.9.1 h1:6QPYqodiu3GuPL+7mfx+NwDdp2eTkp9IfEUpgAwUN0o= github.com/jstemmer/go-junit-report v0.9.1/go.mod h1:Brl9GWCQeLvo8nXZwPNNblvFj/XSXhF0NWZEnDohbsk= github.com/jtolds/gls v4.20.0+incompatible h1:xdiiI2gbIgH/gLH7ADydsJ1uDOEzR8yvV7C0MuV77Wo= github.com/jtolds/gls v4.20.0+incompatible/go.mod h1:QJZ7F/aHp+rZTRtaJ1ow/lLfFfVYBRgL+9YlvaHOwJU= diff --git a/process_testcase.go b/process_testcase.go index ac771bfb..cf68d245 100644 --- a/process_testcase.go +++ b/process_testcase.go @@ -10,6 +10,7 @@ import ( "github.com/fsamin/go-dump" "github.com/ghodss/yaml" "github.com/ovh/cds/sdk/interpolate" + "github.com/pkg/errors" ) var varRegEx = regexp.MustCompile("{{.*}}") @@ -24,6 +25,12 @@ func (v *Venom) parseTestCase(ts *TestSuite, tc *TestCase) ([]string, []string, vars := []string{} extractedVars := []string{} + // the value of each var can contains a double-quote -> " + // if the value is not escaped, it will be used as is, and the json sent to unmarshall will be incorrect. + // This also avoids injections into the json structure of a step + for i := range dvars { + dvars[i] = strings.ReplaceAll(dvars[i], "\"", "\\\"") + } for _, rawStep := range tc.RawTestSteps { content, err := interpolate.Do(string(rawStep), dvars) if err != nil { @@ -32,7 +39,7 @@ func (v *Venom) parseTestCase(ts *TestSuite, tc *TestCase) ([]string, []string, var step TestStep if err := yaml.Unmarshal([]byte(content), &step); err != nil { - return nil, nil, err + return nil, nil, errors.Wrapf(err, "unable to unmarshal teststep") } _, exec, err := v.GetExecutorRunner(context.Background(), step, tc.Vars) @@ -174,6 +181,13 @@ func (v *Venom) runTestSteps(ctx context.Context, tc *TestCase) { vars[k] = content } + // the value of each var can contains a double-quote -> " + // if the value is not escaped, it will be used as is, and the json sent to unmarshall will be incorrect. + // This also avoids injections into the json structure of a step + for i := range vars { + vars[i] = strings.ReplaceAll(vars[i], "\"", "\\\"") + } + var content string for i := 0; i < 10; i++ { content, err = interpolate.Do(string(rawStep), vars) diff --git a/process_teststep.go b/process_teststep.go index 4f841194..cd4973ad 100644 --- a/process_teststep.go +++ b/process_teststep.go @@ -72,7 +72,7 @@ func (v *Venom) RunTestStep(ctx context.Context, e ExecutorRunner, tc *TestCase, tc.computedVerbose = append(tc.computedVerbose, fmt.Sprintf("writing %s", filename)) } - for _, i := range e.Info() { + for ninfo, i := range e.Info() { info, err := interpolate.Do(i, mapResultString) if err != nil { Error(ctx, "unable to parse %q: %v", i, err) @@ -82,7 +82,7 @@ func (v *Venom) RunTestStep(ctx context.Context, e ExecutorRunner, tc *TestCase, continue } filename := StringVarFromCtx(ctx, "venom.testsuite.filename") - info += fmt.Sprintf(" (%s:%d)", filename, findLineNumber(filename, tc.originalName, stepNumber, i)) + info += fmt.Sprintf(" (%s:%d)", filename, findLineNumber(filename, tc.originalName, stepNumber, i, ninfo+1)) Info(ctx, info) tc.computedInfo = append(tc.computedInfo, info) } diff --git a/tests/lib/hello.yml b/tests/lib/hello.yml index fd49b3c1..db166420 100644 --- a/tests/lib/hello.yml +++ b/tests/lib/hello.yml @@ -7,4 +7,5 @@ steps: - result.code ShouldEqual 0 output: display: - hello: "{{.result.systemoutjson.hello}}" \ No newline at end of file + hello: "{{.result.systemoutjson.hello}}" + result: '{{.result.systemout}}' diff --git a/tests/play-with-vars.yml b/tests/play-with-vars.yml new file mode 100644 index 00000000..30f9125d --- /dev/null +++ b/tests/play-with-vars.yml @@ -0,0 +1,37 @@ +name: play-with-vars +vars: + aString: '{"foo": "bar"}' + multipleVars: {"coo": "car"} + multipleVarsSecondSyntax: + goo: gar + aQuote: 'foo"bar' + +testcases: +- name: foobar + steps: + - script: echo '{{.aString}}' + info: value of aString is {{.aString}} + assertions: + - result.systemoutjson.foo ShouldEqual bar + - 'result.systemout ShouldEqual {"foo": "bar"}' + +- name: coocar + steps: + - script: echo '{{.multipleVars.coo}}' + info: value of multipleVars.coo is {{.multipleVars.coo}} + assertions: + - result.systemout ShouldEqual car + +- name: goocar + steps: + - script: echo '{{.multipleVarsSecondSyntax.goo}}' + info: value of multipleVarsSecondSyntax.goo is {{.multipleVarsSecondSyntax.goo}} + assertions: + - result.systemout ShouldEqual gar + +- name: aQuote + steps: + - script: echo '{{.aQuote}}' + info: value of aQuote is {{.aQuote}} + assertions: + - result.systemout ShouldEqual foo"bar \ No newline at end of file diff --git a/tests/user_executor.yml b/tests/user_executor.yml index 20142a13..792ac345 100644 --- a/tests/user_executor.yml +++ b/tests/user_executor.yml @@ -5,17 +5,24 @@ testcases: - type: hello myarg: World assertions: - - result.display.hello ShouldContainSubstring World + - result.display.hello ShouldEqual World + +- name: testAResultDisplay + steps: + - script: echo '{{.testA.result.result}}' + info: value is "{{.testA.result.result}}" + assertions: + - result.systemoutjson.hello ShouldEqual World - name: testB steps: - script: echo "{{.testA.result.display.hello}}" assertions: - - result.systemout ShouldContainSubstring {{.testA.result.display.hello}} + - result.systemout ShouldEqual {{.testA.result.display.hello}} - name: testfoobar steps: - type: foobar bar: hop assertions: - - result.foobar ShouldContainSubstring foo-hop \ No newline at end of file + - result.foobar ShouldEqual foo-hop \ No newline at end of file diff --git a/types.go b/types.go index 7858b7f0..0c9b6300 100644 --- a/types.go +++ b/types.go @@ -173,7 +173,7 @@ type Failure struct { } func newFailure(tc TestCase, stepNumber int, assertion string, err error) *Failure { - var lineNumber = findLineNumber(tc.Classname, tc.originalName, stepNumber, assertion) + var lineNumber = findLineNumber(tc.Classname, tc.originalName, stepNumber, assertion, -1) var value string if assertion != "" { value = color.YellowString(`Testcase %q, step #%d: Assertion %q failed. %s (%v:%d)`, diff --git a/types_executor.go b/types_executor.go index 5aac1377..ef3272cb 100644 --- a/types_executor.go +++ b/types_executor.go @@ -224,6 +224,14 @@ func (v *Venom) RunUserExecutor(ctx context.Context, ux UserExecutor, step TestS if err != nil { return nil, err } + + // the value of each var can contains a double-quote -> " + // if the value is not escaped, it will be used as is, and the json sent to unmarshall will be incorrect. + // This also avoids injections into the json structure of a user executor + for i := range computedVars { + computedVars[i] = strings.ReplaceAll(computedVars[i], "\"", "\\\"") + } + outputS, err := interpolate.Do(string(outputString), computedVars) if err != nil { return nil, err @@ -231,7 +239,7 @@ func (v *Venom) RunUserExecutor(ctx context.Context, ux UserExecutor, step TestS var result interface{} if err := yaml.Unmarshal([]byte(outputS), &result); err != nil { - return nil, errors.Wrapf(err, "unable to unmarshal output") + return nil, errors.Wrapf(err, "unable to unmarshal") } if len(tc.Errors) > 0 || len(tc.Failures) > 0 { return result, fmt.Errorf("failed")