Remaining Python 3 fixes for lib/amo (except lib/git.py) #10598
Conversation
|
Travis didn't agree (well, it was green, but that's because those tests haven't been added to the "working" section yet). I've made more fixes, another round will be necessary but they'll probably be more involved so that'll be in a different PR. |
| @@ -130,7 +130,7 @@ def autograph_sign_data(file_obj): | |||
| signed_manifest = six.text_type(jar.signatures) | |||
|
|
|||
| signing_request = [{ | |||
| 'input': b64encode(force_bytes(signed_manifest)), | |||
| 'input': force_text(b64encode(force_bytes(signed_manifest))), | |||
There was a problem hiding this comment.
Just double checked, this is indeed what we want/need here.
There was a problem hiding this comment.
Yeah. It was broken because the contents weren't str, so that was failing to be encoded in json.
There was a problem hiding this comment.
Also had to do this in the other function in 1bee2d7 (locally this was working, which makes it difficult to track)
diox
changed the title
| assert 'djDebug' not in res.content | ||
| assert 'debug_toolbar' not in res.content | ||
| assert b'djDebug' not in res.content | ||
| assert b'debug_toolbar' not in res.content |
There was a problem hiding this comment.
Let's use force_text(res.content) everywhere maybe?
There was a problem hiding this comment.
Or u'djDebug' not in res.content.decode('utf-8') but we should try to find one style to do this in all our tests primarily
There was a problem hiding this comment.
fwiw, we've used the b'foo' fix a lot in our other test changes (i.e. I'm not saying we shouldn't do this - and I made the same suggestion in another pr - but we're already far down the road of testing for bytestrings in raw .content)
There was a problem hiding this comment.
Yeah, thinking about it, I don't think it matters much anyway. Any way works in regards to testing so 🤷♂️
| @@ -523,8 +523,8 @@ def test_akismet_reports_created_spam_outcome_action_taken(self): | |||
|
|
|||
| validation_response = self.get(self.url(self.guid, '3.0')) | |||
| assert validation_response.status_code == 200 | |||
| assert 'spam' in validation_response.content | |||
| data = json.loads(validation_response.content) | |||
| assert b'spam' in validation_response.content | |||
There was a problem hiding this comment.
should probably use .decode(...) or force_text too here imho. Especially if you're doing it just one line below anyway
|
merging this to make identifying the remaining broken tests easier |
Progress towards fixing mozilla/addons#6323