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

Using more mocks in implicit environ tests. #374

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Jan 5, 2016

This was done mostly because the GCE environment check was taking too long to time-out.


Discovered while investigating #357.


Currently, the four updated tests run in:

$ nosetests \
>   tests/test_client.py:GoogleCredentialsTests.test_get_adc_from_environment_variable_service_account \
>   tests/test_client.py:GoogleCredentialsTests.test_get_adc_from_environment_variable_authorized_user \
>   tests/test_client.py:GoogleCredentialsTests.test_get_adc_from_environment_variable_malformed_file \
>   tests/test_client.py:GoogleCredentialsTests.test_get_application_default_environment_not_set_up
....
----------------------------------------------------------------------
Ran 4 tests in 1.004s

OK

and after this update:

$ nosetests \
>   tests/test_client.py:GoogleCredentialsTests.test_get_adc_from_environment_variable_service_account \
>   tests/test_client.py:GoogleCredentialsTests.test_get_adc_from_environment_variable_authorized_user \
>   tests/test_client.py:GoogleCredentialsTests.test_get_adc_from_environment_variable_malformed_file \
>   tests/test_client.py:GoogleCredentialsTests.test_get_application_default_environment_not_set_up
....
----------------------------------------------------------------------
Ran 4 tests in 0.005s

OK

This was done mostly because the GCE environment check
was taking too long to time-out.
@dhermes dhermes force-pushed the mock-environ-in-test-client branch from 9409866 to 6741b1e Compare January 5, 2016 07:57
@nathanielmanistaatgoogle
Copy link
Contributor

Don't the tests now run faster because they're executing less code? It looks like without this change the tests go all the way "through" oauth2client and hit the "test circumstances" "below" oauth2client, but with this change the mocking is done within oauth2client and the tests execute less production code than before.

Sorry for all the weird quotes; I don't talk about this sort of thing often so I don't have a strong vocabulary of concepts for it.

Since I know you're minding test coverage I know that you haven't uncovered any production code, but isn't there an argument to be made for redundant and overlapping test coverage?

How easily could I propose a guideline that oauth2client's tests should never mock out any part of oauth2client? One should never mock out any part of the system under test, right? So then it becomes a philosophical question of the extent to which test_module.py tests module.py and the extent to which it tests oauth2client, right?

@dhermes
Copy link
Contributor Author

dhermes commented Jan 5, 2016

tests should never mock out any part of oauth2client

I disagree. There should be dedicated places to test a function, class or method. If another function, class or method relies on an already tested component, there is no need to re-test that part. Especially in cases where we go 3 or 4 layers deep. At that point, the mocks could begin to be larger than the actual testing code (or even the code under test).

@nathanielmanistaatgoogle
Copy link
Contributor

I'd like to persuade you otherwise at some point but won't make an obstacle of it today. :-) Merging.

nathanielmanistaatgoogle added a commit that referenced this pull request Jan 5, 2016
Using more mocks in implicit environ tests.
@nathanielmanistaatgoogle nathanielmanistaatgoogle merged commit 2421420 into googleapis:master Jan 5, 2016
@dhermes
Copy link
Contributor Author

dhermes commented Jan 5, 2016

Thanks.

@dhermes dhermes deleted the mock-environ-in-test-client branch January 5, 2016 17:58
@dhermes dhermes mentioned this pull request Jan 29, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants