cdent / gabbi

Declarative HTTP Testing for Python and anything else
http://gabbi.readthedocs.org/
Other
148 stars 34 forks source link

Convert file content to text_type #203

Closed tomviner closed 7 years ago

tomviner commented 7 years ago

In fixing a bug in gabbi-tools, I think I found a bug in Gabbi.

It's using str which has different meanings on Python2 and Python3. This was failing on Python2 because it's trying to convert binary data to binary data.

Please have a look at that gabbi-tools PR to see if I can add a test to gabbi test, that would catch this bug. My thought is that _test_data_to_string is an internal function, and therefore, do we need to find a public function to replicate the bug, and write a test against that?

cdent commented 7 years ago

This looks smart, the test you added in hogarthww/gabbi-tools#1 ought to work for this as well, at least in terms of covering things in a functional sense (which is the more common mode of testing in gabbi). If you want to commit one using httpbin as you've done over there, that's fine, or make a change to simple_wsgi. If you do the former I'll probably follow up later with the change to use simple_wsgi instead (so that external network calls are avoided).

tomviner commented 7 years ago

The test in https://github.com/hogarthww/gabbi-tools/pull/1 relies on our custom BodyResponseHandler which calls test._test_data_to_string. So I'd have to install/import/include that handler to make the test work.

cdent commented 7 years ago

Oh duh, of course, sorry, yesterday was a long stupid day. I'll experiment with this a bit later and see if I can come up with a legit idea.

cdent commented 7 years ago

I think #204 is a more complete fix. Experimenting with testing just this change led to failures in python 3. Please have a look and see what you think. Thanks.

cdent commented 7 years ago

Used #204 instead, but without this wouldn't have known to do that one, so thank you very much.