matryer / silk

Markdown based document-driven RESTful API testing.
http://silktest.org/
GNU General Public License v2.0
942 stars 69 forks source link

Improve handling of json bodies during validation #36

Closed felixLam closed 8 years ago

felixLam commented 8 years ago

This uses the type annotation in the expected body to perform a deepEqual comparison on the deserialized JSON.

This PR implements json (defaults to mode=same) and json(mode=subset) as discussed in #25.

I did not have the time yet to add appropriate unit tests, but will happily do so before merging this PR.

felixLam commented 8 years ago

The easiest way to include the new options in the unit tests would be to alter some of the existing ones (e.g. add the json keyword to an expected buddy and pretty print it in the docs, add an unrelated key to the json response and add the subset keyword). I could also add new tests if that is preferred.

matryer commented 8 years ago

Please do add tests before merge.

felixLam commented 8 years ago

@matryer I have added a few failure and success unit tests. These do fail if you omit the json and json(mode=subset) qualifiers.

As you can see the failure tests do provide some insight if the subset test fails: body doesn't match mismatch for key 'meta': missing key client

Let me know what you think.

felixLam commented 8 years ago

I have added a mode=exact as discussed in #25. Let me know if you want subset to be the default.

matryer commented 8 years ago

Awesome - I think just the README needs updating now and we're ready to rock. Great work.

felixLam commented 8 years ago

@matryer my pleasure. I have updated the section about the response validation. https://github.com/matryer/silk/pull/36/commits/a3bf2d91ac14752135bda196fd25eb6950e2c7b8 (edit: i fixed the indentation of the code blocks to show the back ticks correctly) https://github.com/iosphere/silk/blob/feature/jsonChecking/README.md

felixLam commented 8 years ago

@matryer Anything else you would like me to change before this PR can be merged?

felixLam commented 8 years ago

@matryer any news on this PR? Would love to see this in the main repo.

matryer commented 8 years ago

json(mode=exact) is the same as no tag at all right? We shouldn't parse the JSON if they don't put the json tag on. Then, perhaps json could be the default flexible option, and json(strict) could disallow additional fields?

Only a superficial change, but makes the API simpler. What do you think @felixLam?

felixLam commented 8 years ago

@matryer yes, json(mode=exact) is the same as no tag. We could certainly drop this. json without mode is already the flexible option. We can of course rename same to strict and drop the exact mode. I will check if it is useful/necessary to have the mode prefix.

matryer commented 8 years ago

Great @felixLam - thanks. I think, with those tweaks, that's a great addition to the tool.

felixLam commented 8 years ago

@matryer I made the suggested changes and updated both the README and test files.