New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Python 3 binary vs unicode integration test issues #6724

Merged
merged 3 commits into from Nov 3, 2018

Conversation

Projects
None yet
2 participants
@Eric-Arellano
Contributor

Eric-Arellano commented Nov 3, 2018

No description provided.

@Eric-Arellano Eric-Arellano requested a review from stuhood Nov 3, 2018

@@ -57,7 +57,8 @@ def fingerprint_file(workspace, filename):
"""
content = read_file(os.path.join(workspace, filename))
fingerprint = hashlib.sha256(content)
return 'sha256={}'.format(base64.b64encode(fingerprint.digest())), str(len(content))
b64_encoded = base64.b64encode(fingerprint.digest())
return 'sha256={}'.format(b64_encoded.decode('utf-8')), str(len(content))

This comment has been minimized.

@Eric-Arellano

Eric-Arellano Nov 3, 2018

Contributor

This wouldn't fail, but also wouldn't behave as expected.

In Py3:

[In]: '{}'.format(b"hello")
[Out]: 'b"hello"'

That is, the bytes prefix is added, which we don't want, so we must decode back to unicode.

@stuhood

stuhood approved these changes Nov 3, 2018

@stuhood

This comment has been minimized.

Member

stuhood commented Nov 3, 2018

Thanks @Eric-Arellano.

Do we have a strategy yet for getting test coverage for 3 integration? It's good to make clear-cut fixes that continue to pass under 2, though.

@Eric-Arellano

This comment has been minimized.

Contributor

Eric-Arellano commented Nov 3, 2018

Thanks @stuhood for the reviews!

We don't yet have an agreed upon strategy. I'll review what we discussed over the summer, and present a few options to the Python 3 slack channel.

@Eric-Arellano Eric-Arellano merged commit 0f71717 into pantsbuild:master Nov 3, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Eric-Arellano Eric-Arellano deleted the Eric-Arellano:fix-temporary-file-content branch Nov 3, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment