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

Python 3 fixes - test root unicode vs bytes #6253

Merged
merged 5 commits into from Jul 28, 2018

Conversation

Projects
None yet
2 participants
@Eric-Arellano
Copy link
Contributor

Eric-Arellano commented Jul 27, 2018

We have to decide if we want create_file() to still default to bytes instead of unicode.

It has API public, so I think it's not really open to discussion, right?

If it's open to discussion, we may want to go back to defaulting to unicode, as most use cases seem to use unicode and this would correspond to the default way of opening files.

Otherwise not a big deal, and the below fixes the default call to create_file() from breaking.

@Eric-Arellano Eric-Arellano changed the title Python 3 fixes - test_base unicode being written to binary file Python 3 fixes - test root unicode vs bytes Jul 28, 2018

@@ -274,7 +274,7 @@ def run_pants_with_workdir_without_waiting(self, command, workdir, config=None,
env['PANTS_PROFILE'] = prof
# Make a note the subprocess command, so the user can correctly interpret the profile files.
with open('{}.cmd'.format(prof), 'w') as fp:
fp.write(b' '.join(pants_command))
fp.write(' '.join(pants_command))

This comment has been minimized.

@Eric-Arellano

Eric-Arellano Jul 28, 2018

Contributor

pants_command is in unicode and file opened in text_mode, so I figured it makes most sense to stay with text.

@stuhood
Copy link
Member

stuhood left a comment

With regard to create_file: the default empty value is equivalent as bytes and as unicode, and it's never concatenated with anything in memory. So should be fine either way.

Thanks!

@stuhood stuhood merged commit 54e6848 into pantsbuild:master Jul 28, 2018

1 check passed

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

@Eric-Arellano Eric-Arellano deleted the Eric-Arellano:py3-fixes_test-base-binary-files branch Jul 28, 2018

CMLivingston pushed a commit to CMLivingston/pants that referenced this pull request Aug 27, 2018

Python 3 fixes - test root unicode vs bytes (pantsbuild#6253)
* Fix unicode being written to bytes file

* Fix binary vs unicode issues in rest of test root

* Fix one more binary.format() issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment