Skip to content
This repository has been archived by the owner on Nov 5, 2019. It is now read-only.

Re-enable Python3 tests and fixing Py2/Py3 issues in tests. #87

Merged
merged 1 commit into from
Dec 8, 2014

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Dec 6, 2014

Partially fixes #85.

Still need to re-enable the 5 tests which make HTTP requests.

@dhermes
Copy link
Contributor Author

dhermes commented Dec 6, 2014

I should mention that just because these tests pass does not mean the Py3 support is in good shape.

On the contrary, it is really bad.

For example:

  • Almost all of the strings are still 'val' without declaring bytestring (Py2) or unicode (Py3). Hence they are a different type from one to the other.
  • The convenience method of casting to a string str(e) or '%s' % val is used without an idea of what the value actually is. In Py3, str(b'a') == "b'a'". It's very clear just from the changes I had to make that the most parts of the code assume Python2 strings.

Also, it seems to me that one of the reasons the tests seem slow is file I/O. There is a nontrivial amount of reading from the DATA_DIR which could be replaced with a module containing the actual content. I somewhat understand wanting tests to actually make sure that file I/O is working, but it is so low-level it isn't really necessary to test (though open(filename) with no mode actually happens in this library 😢).

I don't think the people who gave Python3 support read this:
https://docs.python.org/3/howto/pyporting.html

craigcitro added a commit that referenced this pull request Dec 8, 2014
Re-enable Python3 tests and fixing Py2/Py3 issues in tests.
@craigcitro craigcitro merged commit b047471 into googleapis:master Dec 8, 2014
@craigcitro
Copy link
Contributor

yeah, i'm sure there's a lot to be done for better py3 support. the current support does seem to be much better than what we had before (i.e. none at all ;)).

@dhermes
Copy link
Contributor Author

dhermes commented Dec 8, 2014

@craigcitro I am legitimately concerned with the quality of the code, not just "ew this is gross" but actual places where no one is clear on the behavior. Just my two cents.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test_oauth2client does not get run
2 participants