search

aviaviavi / curl-runnings

A declarative test framework for quickly and easily writing integration tests against JSON APIs.
MIT License
157 stars 13 forks source link

Adds support for URLEncoded request bodies on the requestData field #52

Closed paulcadman closed 4 years ago

paulcadman commented 4 years ago

To set a URLEncoded body set bodyType to urlencoded:

  requestData:
     bodyType: urlencoded
     content:
       a: b
       c: d
       e: f

To set a JSON body set bodyType to json:

  requestData:
     bodyType: json
     content:
       a: b
       c: d
       e: f

In either case the body is formed from the value of the content key.

The change is backwards compatible so that the following:

  requestData:
    a: b
    c: d
    e: A

will send a JSON body formed from the value of the requestData key.

I'd welcome comments on the design. I'd like to add some tests and update the examples before merging.

aviaviavi commented 4 years ago

@paulcadman Overall, this change sounds great to me. Concerning the design, my only point of feedback would be that it would be ideal if bodyType were optional so that it didn't need to be specified every time. I figure lots of test suites will only use a single bodyType.

I'd be fine with either defaulting to JSON when it's not specified, or even accepting something like a defaults.bodyType at the top level of the test spec, what do you think? I assume the former is easier to implement.

paulcadman commented 4 years ago

Hi @aviaviavi - thanks for the feedback.

I agree that bodyType should be optional. The current implementation does default to JSON if no bodyType is specified. So most existing specifications (except ones that contain a bodyType key) will continue to work as before.

Of course, if a JSON payload does contain a bodyType key itself - it can still be specified by using the bodyType: json and contents: option.

aviaviavi commented 4 years ago

Ok cool sounds good then! I might be misreading the parser code slightly, will add a comment there directly.

aviaviavi commented 4 years ago

Ahh ok, I was mistaken. I was thinking we'd want

  requestData:
     content:
       a: b
       c: d
       e: f

to parse the same as

  requestData:
       a: b
       c: d
       e: f

but when I thought through it more fully, realized we definitely dont want that. so I'm fully on board with what you have! Let me know when you get the tests and docs in, and I'll do a full review again.

paulcadman commented 4 years ago

Hi @aviaviavi - could you have a look at this again? I've added documentation and tests.