Support testing with non text response bodies (e.g. images)

[banks]

Hi,

I have a possibly unusual use-case for this library: testing some very complex redirect/proxying logic that is translating a complicated URL structure for one thumbnail generating service to another we want to use.

I've used pyresttest for similar things before and would really like to again to have robust suite of validation for every type of URL we must support.

The problem is that the new proxy, when it works, is returning image data. If the test passes all is OK provided we make no assertions based on the body content, but in the common case where it fails this line: https://github.com/svanoort/pyresttest/blob/ae0a1215696958726a61cd2e7e9baaf8ff08baa2/pyresttest/resttest.py#L431

    # Print response body if override is set to print all *OR* if test failed
    # (to capture maybe a stack trace)
    if test_config.print_bodies or not result.passed:
        if test_config.interactive:
            print("RESPONSE:")
        print(result.body.decode(ESCAPE_DECODING))

Causes a problem - the decode call throws exceptions trying to decode binary data.

I can think of a couple of possible solutions:

1. (work around) change it so that if --print-bodies is explicitly set to false rather than just unset, this line is skipped
2. Detect Content-Type of response and only try to print it if it's a known-text format (i.e. text/* or application/json, maybe find a white list of common ones?
3. Detect Content-Type of response and black-list common, known-binary types like image/* and application/octet-stream etc.
4. Create a new per-test config that allows you to specify this explicitly, perhaps expect_binary_body: true

Which of the above (or another alternative) is preferred? If there is a response soon enough I can probably provide a PR implementing any of those.

Thanks for the great tool!

[svanoort]

@banks Thank you for reporting this and giving the detail. If you can put together a PR addressing this, it would certainly be welcome and I'd be happy to review/test/merge as appropriate. I am aiming to set aside more time in the next few weeks for PyRestTest (free time tends to come in bursts).

This bug was definitely an unintended consequence, here are my thoughts on your solutions:

1. I don't like this option, because it is tied to the current implementation, and the whole mechanism behind them will changing massively with #101
2. This one is smart, and I think the best option, given that the result object created during execution has a field for headers. I'd probably match on text/, *xml, json, and then go from there. In cases where options would print a body but it does not match one of these, I would print a line like "Response body not printed: content-type XXX/YYY does not match patterns ['text/_', 'xml', '_json*'] so assumed to be binary."
3. Too many different MIME types to blacklist, it will just get messy.
4. This is a thought, but I really want to avoid adding too many configuration options here, and rely on sensible defaults.

I think the best solution is a combination:

1. Create a function for this, that does the printing and is invoked during execution (and is easy to unit test alone).
2. Define a list of regex patterns within the resttest.py class: PRINTABLE_CONTENTTYPE_PATTERNS which are matched against content-type
3. Function does a comparison against pattern list, if it is in the list, print the body, otherwise print the message above about it probably being binary (and doing a string-formatted output of the patterns list).
4. If an exception is thrown during decoding of body for printing, catch the exception and print a message like "Failed to print body due to {Exception type}"
5. Remember we have to handle both raw bytes and Unicode, on Python 2.6, 2.7, and 3.x. There are helper functions in place to simplify this already.

I think we're getting far enough along that if a PR for this + cookie extractor are merged in, it might be time to cut another small release (and to heck with quirks in version numbering of milestones). Potentially merging in my execution/logging refactors if I can, since otherwise future work will explode with merge conflicts.

Anyway, looking forward to hearing your thoughts (possibly in PR form).

Cheers! Sam

[banks]

Thanks for the response Sam,

Your proposal makes a lot of sense and was my preferred option too.

I will see how this week pans out - I'm in an interesting position right now where I'm actually handing this work on to another team in the next couple of weeks so it's not clear if I'll have time to work on a PR for this or not before I'm no longer working on the project.

It sounds like it's not a huge amount of work but at least a little overhead to refactor and add suitable tests etc. I will likely know in next few days if this is likely to be possible for me to do or not and let you know (unless you have a moment and inclination to work on it yourself).

Thanks for the great tool

[svanoort]

@banks Absolutely, I know how that work gets shuffled around. I'll probably do a quick fix for the immediate problem (catch exceptions when printing body and log that it tried to print the body and failed), so your tests don't bomb out while running. If you can slip in a PR with content handling, that's wonderful and welcomed, if not, I'll convert this to a feature request to work on soon.

Anyway, glad you've been finding PyRestTest quite useful, and think you'll like where it's going in the future ;)