Catch an edge case in imports.

Catch an edge case in imports.

[dhermes, 2013-03-01 16:33:32]

https://codereview.appspot.com/7454047/diff/4001/Makefile
File Makefile (right):

https://codereview.appspot.com/7454047/diff/4001/Makefile#newcode8
Makefile:8: cd python2 && python2.7 httplib2test_appengine_aberrant.py
Still need to run
python2.7 httplib2test_appengine.py

Also, kill the tab char (red arrow).

https://codereview.appspot.com/7454047/diff/4001/python2/httplib2test_appengi...
File python2/httplib2test_appengine_aberrant.py (right):

https://codereview.appspot.com/7454047/diff/4001/python2/httplib2test_appengi...
python2/httplib2test_appengine_aberrant.py:4: A set of unit tests for
httplib2.py on Google App Engine
Can you make this docstring accurate?

https://codereview.appspot.com/7454047/diff/4001/python2/httplib2test_appengi...
python2/httplib2test_appengine_aberrant.py:30:
google.appengine.api.apiproxy_stub_map = None
Can you do this in setUp and fix it in tearDown?

[jcgregorio_google, 2013-03-01 16:58:18]

https://codereview.appspot.com/7454047/diff/4001/Makefile
File Makefile (right):

https://codereview.appspot.com/7454047/diff/4001/Makefile#newcode8
Makefile:8: cd python2 && python2.7 httplib2test_appengine_aberrant.py
On 2013/03/01 16:33:32, dhermes wrote:
> Still need to run
> python2.7 httplib2test_appengine.py
> 
> Also, kill the tab char (red arrow).

Done.

https://codereview.appspot.com/7454047/diff/4001/python2/httplib2test_appengi...
File python2/httplib2test_appengine_aberrant.py (right):

https://codereview.appspot.com/7454047/diff/4001/python2/httplib2test_appengi...
python2/httplib2test_appengine_aberrant.py:4: A set of unit tests for
httplib2.py on Google App Engine
On 2013/03/01 16:33:32, dhermes wrote:
> Can you make this docstring accurate?

Done.

https://codereview.appspot.com/7454047/diff/4001/python2/httplib2test_appengi...
python2/httplib2test_appengine_aberrant.py:30:
google.appengine.api.apiproxy_stub_map = None
Not really, as it has to be done before httplib2 is imported.

On 2013/03/01 16:33:32, dhermes wrote:
> Can you do this in setUp and fix it in tearDown?

[dhermes, 2013-03-01 17:04:15]

https://codereview.appspot.com/7454047/diff/4001/python2/httplib2test_appengi...
File python2/httplib2test_appengine_aberrant.py (right):

https://codereview.appspot.com/7454047/diff/4001/python2/httplib2test_appengi...
python2/httplib2test_appengine_aberrant.py:30:
google.appengine.api.apiproxy_stub_map = None
Ah yes, good call.

Since you're testing the import, can you do that in the test itself?

The reason for the concern, the module 'google.appengine.api' will live in
sys.modules and could have unintended side effects on other tests that get run.
It is very easy to restore this in tearDown, so there is no harm in doing this.

On 2013/03/01 16:58:18, jcgregorio_google wrote:
> Not really, as it has to be done before httplib2 is imported.
> 
> On 2013/03/01 16:33:32, dhermes wrote:
> > Can you do this in setUp and fix it in tearDown?
>

https://codereview.appspot.com/7454047/diff/14001/python2/httplib2test_appeng...
File python2/httplib2test_appengine_aberrant.py (right):

https://codereview.appspot.com/7454047/diff/14001/python2/httplib2test_appeng...
python2/httplib2test_appengine_aberrant.py:2: httplib2test_appengine
This isn't httplib2test_appengine, it's httplib2test_appengine_aberrant

[jcgregorio_google, 2013-03-01 19:20:22]

https://codereview.appspot.com/7454047/diff/4001/python2/httplib2test_appengi...
File python2/httplib2test_appengine_aberrant.py (right):

https://codereview.appspot.com/7454047/diff/4001/python2/httplib2test_appengi...
python2/httplib2test_appengine_aberrant.py:30:
google.appengine.api.apiproxy_stub_map = None
Done. Moved the test back to httplib2test_appengine.py.

On 2013/03/01 17:04:15, dhermes wrote:
> Ah yes, good call.
> 
> Since you're testing the import, can you do that in the test itself?
> 
> The reason for the concern, the module 'google.appengine.api' will live in
> sys.modules and could have unintended side effects on other tests that get
run.
> It is very easy to restore this in tearDown, so there is no harm in doing
this.
> 
> On 2013/03/01 16:58:18, jcgregorio_google wrote:
> > Not really, as it has to be done before httplib2 is imported.
> > 
> > On 2013/03/01 16:33:32, dhermes wrote:
> > > Can you do this in setUp and fix it in tearDown?
> > 
>

[dhermes, 2013-03-02 00:30:35]

LGTM

[jcgregorio_google, 2013-03-04 01:39:11]

On 2013/03/02 00:30:35, dhermes wrote:
> LGTM

Committed in
https://code.google.com/p/httplib2/source/detail?r=a645d22abfd0cc724f61c8f6c3...