Add coverage for 'gcloud._apitools.transfer'

[tseaver]

Apologies: this PR is a ginormous pile of tests, with more cleanups than the earlier ones.

Looking forward to the sound of the chainsaw to get rid of deadwood now that coverage is complete.

N.B.: due to https://github.com/nose-devs/nose/blob/00ae0930e3bfad41279a0899e8c699f2ac1ff209/nose/loader.py#L164, the default tox -e cover misses all these tests. I believe I'm going to have to rename the sub-package (gcloud/_apitools -> gcloud/streaming, maybe). Workaround is to run the following after tox -e cover:

$ .tox/cover/bin/nosetests  \
   --with-coverage   --cover-package=gcloud   --cover-tests   --cover-branches   \
   gcloud/_apitools
......................................................................................................................................................................................................................
Name                                       Stmts   Miss Branch BrPart  Cover   Missing
--------------------------------------------------------------------------------------
gcloud.py                                      3      0      0      0   100%   
gcloud/_apitools.py                            0      0      0      0   100%   
gcloud/_apitools/buffered_stream.py           27      0      4      0   100%   
gcloud/_apitools/exceptions.py                41      0      0      0   100%   
gcloud/_apitools/http_wrapper.py             154      0     70      0   100%   
gcloud/_apitools/stream_slice.py              22      0      4      0   100%   
gcloud/_apitools/test_buffered_stream.py      93      0      0      0   100%   
gcloud/_apitools/test_exceptions.py           65      0      0      0   100%   
gcloud/_apitools/test_http_wrapper.py        584      0     30      0   100%   
gcloud/_apitools/test_stream_slice.py         58      0      0      0   100%   
gcloud/_apitools/test_transfer.py           1693      0     34      0   100%   
gcloud/_apitools/test_util.py                 52      0      4      0   100%   
gcloud/_apitools/transfer.py                 445      0    180      0   100%   
gcloud/_apitools/util.py                      25      0     14      0   100%   
gcloud/_testing.py                            12      0      4      0   100%   
--------------------------------------------------------------------------------------
TOTAL                                       3274      0    344      0   100%   
----------------------------------------------------------------------
Ran 214 tests in 0.055s

[dhermes]

Please don't add code with an XXX in it. If you want to make a TODO, use GitHub's world class issue tracking system.

[tseaver]

I've pulled the XXX out in #1210, along with the other pep8 / pylint cleanups.

[tseaver]

Note that the comments (w/o the XXX) needs to stay in the code, FBO the next unfortunate soul who stumbles across that bit of RFC abuse.

[dhermes]

This is becoming quite cumbersome to review. Why couldn't you fold the changes into this PR?

[tseaver]

@dhermes can you get past the OCD about the XXX? When pylint gets enabled (in #1210), it will all come out in the wash, along with the numerous existing TOOD comments already there. I'd really like not to have to pre-lint the code here, and then deal with the merge/rebase conflicts in the PR where I've already fixed them.

Or we could blow off this PR (and #1209) and just go to reviewing #1210 instead.

[jgeewax]

I'm +1 to pulling out the XXX's and moving them to an issue. Right now, it's unclear to anyone else what the meaning of "XXX" is here, which means that the bus-factor (what happens if you get hit by a bus) comes into play. It looks like there's only 3 issues to create anyway, right? if so, let's just do that.

[tseaver]

@jgeewax they get pulled in #1210 (when the linting is turned on). I'm hoping to just get past this one (and #1208) as "mechanical" issues, and focus in #1209 (stripping features) and #1210 (pep8/linting fixes), without having to merge changes forward.

[jgeewax]

Why merge to master then? Why not do this in a GoogleCloudPlatform branch and then when lint is on it gets into master..>?

[tseaver]

Why merge to master then? Why not do this in a GoogleCloudPlatform branch and then when lint is on it gets into master?

Maybe that would have been better, but.... We have already merged the forked apitools code to master, and it contains tons of lint violations (TODO comments, wildly-non-conforming names, etc.). We actually have code in place to turn off pep8/pylint/coverage on this directory.

The plan was to get full test coverage, then clean up and pare down the code. This PR is really about test coverage: the "XXX" comments are a result of my outrage from trying to understand / cover the code.

I guess we could revert #1189, #1190, #1191, #1191, #1192, #1193, and close this one, and re-do them against a revendor_apitools branch, along with re-pointing #1208, #1209, and #1210 to it. That seems like a lot of effort to avoid allowing some harmless comments on master for the hours/day it will take to get #1208, #1209, and #1210 reviewed.

[tseaver]

In case it isn't clear, here are the relevant changes from #1210:

https://github.com/tseaver/gcloud-python/commit/dbd128472f1ffdb3e30a4000e6996745e4608fab#diff-c22973d2670fd481a1185c937a311decL649
https://github.com/tseaver/gcloud-python/commit/dbd128472f1ffdb3e30a4000e6996745e4608fab#diff-c22973d2670fd481a1185c937a311decL684
https://github.com/tseaver/gcloud-python/commit/dbd128472f1ffdb3e30a4000e6996745e4608fab#diff-c22973d2670fd481a1185c937a311decL713

[tseaver]

@dhermes can re-start here, and defer the "lint" bits until #1210?

[dhermes]

Just merge it.

We've got to have a plan going forward to avoid situations like this.

[tseaver]

ISTM that an integration branch is the right long-term strategy for changes which we want to break up into multiple PRs (for ease of review) but not land on master until they are all ready.

[dhermes]

Sure thing