Support for http methods via http.method and http.request-body

[ablancolopez]

Checklist

• make test-all (UNIX) passes. CI will also test this
• unit and/or integration tests are included (if applicable)
• documentation is changed or added (if applicable)

Unable to pass integration tests, even before including my changes. Did several manual tests instead.

Description of change

This PR adds support for HTTP verbs. In order to achieve this, two fields have been added, so it ends like this:

http:
  https://httpbin.org/put:
    status: 200
    method: PUT
    request-body: '{"key": "value"}'
    body: 
      - '"key": "value"'

Those are not breaking changes, because when no method is provided, http.NewRequest defaults to GET.

Relates to #484

[aelsabbahy]

A quick glance at the code/tests added and this looks great. Haven't tried it locally yet.

The only thing missing is url, I feel that's important to this feature.

For example, if one needs to test the same URL with different methods they won't be able to

http:
  https://someurl.com:
    status: 200
    allow-insecure: false
    no-follow-redirects: false
    timeout: 5000
    body: []
  https://someurl.com:
    status: 200
    method: PUT
    request-body: "blah"
    allow-insecure: false
    no-follow-redirects: false
    timeout: 5000
    body: []

Command.exec does this today. See: #431 (issue) and #434 (code)

That all said, I'm fine with that being a follow up PR. Let me know if it's something you're interested in working on.. Otherwise, I'll get to it before I release this feature.

Many thanks for this, great enhancement!

@sshipway will this close #484 for you?

[ablancolopez]

Agree that url should be added before releasing this feature. I will start working on it following the examples you provided and adding some tests in order to cover that example.

Thanks for the feedback!

[ablancolopez]

All done @aelsabbahy!

codeclimate complains about the receiver name not matching ID method, but I think it is better to keep using r to match GetTitle and GetMeta methods receiver name. Have no strong opinion tho.

Hope it now fits better the scope for #484

[aelsabbahy]

Looks great! The whole u/r needs to be consistent, but I can follow up with a PR that resolves it since there are inconsistencies across resources/*.go currently.

Gave it a quick test and it looks good.. minor change requested, then I'll do a more thorough test, but not anticipating any issues given the tests you've added.

[aelsabbahy]

This makes the output inconsistent with command. Let's exclude the URL for now when url is specified .

I'm open to discussing this further, feel free to open up an issue dedicated to this if you feel strongly about it one way or another.

Just opting for consistency then we can follow up with a PR that changes that to another consistent pattern.

[ablancolopez]

Oh, didn't take a look at the current source code and based mine on #434 changes. Modifying it

[sshipway]

Having a 'url' option to the http test is definitely a Good Thing and matches up with the exec tests.
Being able to specify a method and request-body allows simple tests for REST APIs, so can close #484 I think, though I was hoping to be able to specify the request-body as a hash (which is converted to JSON) as well as a string, and have a test in 'body' that is a hash of value/pair tests as well as just string matches. However it would be a lot fo work to do this, and it could be done as a future feature req...
Many thanks @blalop for adding the features that I was too busy/lazy/bad-at-coding to make work myself!

[aelsabbahy]

@sshipway Thanks for the response!

Request body hash -> JSON is a little too opinionated for my taste, maybe that's something we can revisit later if there's still a need.

Response validation is something I'm very interested in and I'm currently working on it for v0.4.0.

Let me know what you think of this: #576 (comment)

Any feedback is good, I think that PR is getting close to completion.

[sshipway]

I like #576, being able to use a gjson test against a string (assuming the string is json of course). I would prefer a more terse output by default, with an option for detailed logs.

If we have the ability to match returned values using json, then it would seem to me to be a logical extension to allow the option to somehow define the body as json (via a hash), but that's more for convenience in testing REST APIs, can always achieve the same using a formatted string. Maybe have body-json and body-formencoded attributes as hash alternatives to body, instead of overloading the body attribute? Much of the reason I prefer having the body defined as a hash is that it allows overriding of selected parameters when managed via heira in puppet, though I acknowledge that this is a very specific use case

[aelsabbahy]

Looks great!

[aelsabbahy]

@sshipway the whole map to JSON seems a little too specific/opinionated to be honest and I don't plan on supporting something like that in the near term. I'll let you know if that changes.

Hoping this change + #576 allows you to leverage goss in more ways =)

@blalop I'll merge/release this sometime this week. But everything looks good to go!

[aelsabbahy]

Curious, did you merge master into this manually or was this an automated process?

[ablancolopez]

It was me, happened to be around here when the conflicts showed :)

btw, do you know when this PR could be merge?

[aelsabbahy]

I was working on resolving the conflict locally, then I look at the PR and saw your update.. thought I was losing my mind =)

I'm going to merge it now, but might not be released for another day or so. There's another PR or two that I'm trying to put in the next release.

Thanks for the contribution, this change is awesome!