Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test case interpolation incorrectly replaces "-" with "µµµ" #475

Closed
sborsje opened this issue Dec 21, 2021 · 2 comments · Fixed by #479
Closed

Test case interpolation incorrectly replaces "-" with "µµµ" #475

sborsje opened this issue Dec 21, 2021 · 2 comments · Fixed by #479
Assignees
Labels

Comments

@sborsje
Copy link

sborsje commented Dec 21, 2021

String interpolation seems to break in a very specific edge-case.

I've been able to create a minimal test suite to reproduce the problem in Venom 1.0.0, 1.0.1, and the latest from master (1.0.0-rc.7 and older don't have this issue):

---
name: Interpolation POC
vars:
  accept: "text/html"

testcases:
  - name: GetVar
    steps:
      - type: http
        method: GET
        url: "http://example.com"
        assertions:
          - result.statuscode ShouldEqual 200
        vars:
          accept:
            from: result.headers.content-type

  - name: UseVar
    steps:
      - type: http
        method: GET
        url: "http://example.com"
        headers:
          test: "{{ .GetVar.accept }}"
        assertions:
          - result.statuscode ShouldEqual 200
          - result.headers.x-cache ShouldEqual "HIT"
      - type: http
        method: GET
        url: "http://example.com"
        headers:
          test: "{{ .GetVar.accept }}"
        assertions:
          - result.statuscode ShouldEqual 200
          - result.headers.accept-ranges ShouldEqual "bytes"

This yields the following result:

 • Interpolation POC (/tests/interpolation.yml)
 	• GetVar SUCCESS
 	• UseVar FAILURE
Testcase "UseVar", step #1: Assertion "result.headers.acceptµµµranges ShouldEqual \"bytes\"" failed. expected: bytes  got: <nil> (/tests/interpolation.yml:0)

A few things to note:

  • In the UseVar testcase, there are 2 steps defined. They both execute the same HTTP request, but look at different headers from the result.
  • The first step succeeds, but the second step fails. Note that the assertion tries to look at the acceptµµµranges header, which is incorrect.
  • As far as I can tell, this only happens when the test case refers back to a result from a previous test case. If in the example above we replace {{ .GetVar.accept }} with {{ .accept }} (which is defined in the vars key at the top), everything works fine.

In our codebase we use this pattern to execute a login request to generate a JWT token at the beginning of the suite, which we then use as a Authorization header in subsequent testcases.

I've tried to debug this a little bit, and was able to trace back the string interpolation to the following function:
https://github.com/ovh/cds/blob/c80557c58fbb2fc28af3da94385172abefddc9fc/sdk/interpolate/interpolate.go#L49

The Do function yield different output for both test cases when it's called from: https://github.com/ovh/venom/blob/master/process_testcase.go#L209

I'm not really sure where to go from here, because both test cases have similar raw input, yet a very different vars slice. My hunch is that something in the vars slice trips up the interpolation function?

@fsamin fsamin added the bug label Dec 27, 2021
@fsamin fsamin self-assigned this Dec 27, 2021
@fsamin
Copy link
Member

fsamin commented Dec 27, 2021

Thanks for the detailed report.
This is related to a bug in the interpolation lib.
I've just made a PR on this lib to fix it: ovh/cds#6047. I have been able to confirm that the PR fixes your testcase.

I will make a PR on venom once the PR above will be merged on master branch

fsamin added a commit that referenced this issue Dec 27, 2021
fsamin added a commit that referenced this issue Dec 27, 2021
fsamin added a commit that referenced this issue Dec 27, 2021
fsamin added a commit that referenced this issue Dec 27, 2021
@sborsje
Copy link
Author

sborsje commented Dec 27, 2021

Awesome! Thanks, @fsamin! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants