open() files in binary if we're going to write bytes

[glasserc]

This is meant to address #79. I'm not sure if there are other Python 3-related issues around this project -- this is just the first one we hit. The tests didn't catch it because they substitute a mock for the open() call.

[peterbe]

Can we stop mocking the amo2kinto.generator.open and instead fiddle so it writes to files in a temp place that gets cleaned up.

I don't understand "other Python 3-related issues". This project surely requires Python 3. The .travis.yml and the setup.py seems to indicate so.

Perhaps now's not the time but the setup.py says this project suppose 3.5 and 3.6 but the travis test only test 3.6.

[peterbe]

Nit about not bothering to convert it to a byte string and keeping 'w'.

The biggest culprit here is that the tests should not mock open and instead be allowed to write a file and the tests should open the file to check if they're sane.

[peterbe]

res is a unicode string so it's correct to use wb if you're going to convert it to a byte string. But I would instead keep it as a unicode string.

I.e.

res = '🚵‍♂️'
with open('/tmp/index.html', 'w') as f:
    f.write(res)

This works.

[glasserc]

Why do you prefer this? It seems natural to me to write bytes to disk, since after all a disk contains a string of bytes, not a string of code points.

[glasserc]

Can we stop mocking the amo2kinto.generator.open [...] The biggest culprit here is that the tests should not mock open and instead be allowed to write a file and the tests should open the file to check if they're sane.

No, the unit tests are correct to mock/inject the dependencies of the code under test. I have opened #81 to see about adding a functional test.

I don't understand "other Python 3-related issues". This project surely requires Python 3. The .travis.yml and the setup.py seems to indicate so.

"Requires Python 3" is another way of saying "is not compatible with Python 2". It is not the same thing as "functions under Python 3".

Perhaps now's not the time but the setup.py says this project suppose 3.5 and 3.6 but the travis test only test 3.6.

What do you mean? Both Python 3.5 and 3.6 are visible on the Travis results for this PR.

[peterbe]

Regarding the Travis only running 3.6. I read the file wrong. My bad.