Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

@dhermes
Copy link
Contributor

@dhermes dhermes commented Feb 20, 2015

NOTE: Has #668 as diffbase

Tests previously were mixing the logic of the 4 possible environments while the simple ordering of them in set_default_dataset_id() and now _determine_default_dataset_id().

This PR separates that logic and makes sure each environ helper is called in a specific order.

In order to do this it also introduces two one-line functions _get_production_dataset_id() and _get_gcd_dataset_id() in the implicit environ module.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 20, 2015
@dhermes dhermes added api: datastore Issues related to the Datastore API. testing labels Feb 20, 2015
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling a09391e on dhermes:fix-_implicit_environ-tests into 56f60bb on GoogleCloudPlatform:master.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

Also defining one-liner _get_production_dataset_id() and
_get_gcd_dataset_id() methods to mirror the app_engine_id()
and compute_engine_id() methods in _implicit_environ.
Also fixing unused dataset_id=None in _callFUT in
Test__get_gcd_dataset_id and Test__get_gcd_dataset_id.
Also adding the relative ordering parts to Test__determine_default_dataset_id.

Tests previously were mixing the logic of the 4 possible environments
while the simple ordering of them in set_default_dataset_id() and
now _determine_default_dataset_id().

This separates that logic and makes sure each environ helper is called
in a specific order.
@dhermes dhermes force-pushed the fix-_implicit_environ-tests branch from a09391e to 1dd6709 Compare February 25, 2015 02:14
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 1dd6709 on dhermes:fix-_implicit_environ-tests into 0e41dc3 on GoogleCloudPlatform:master.

@dhermes
Copy link
Contributor Author

dhermes commented Feb 25, 2015

@tseaver Issues remaining?

@tseaver
Copy link
Contributor

tseaver commented Feb 25, 2015

LGTM

dhermes added a commit that referenced this pull request Feb 26, 2015
Unwinding cases in Test_set_default_dataset_id and having better test separation
@dhermes dhermes merged commit d43c4a0 into googleapis:master Feb 26, 2015
@dhermes
Copy link
Contributor Author

dhermes commented Feb 26, 2015

Let's see if Travis is all the way back on track...

@dhermes dhermes deleted the fix-_implicit_environ-tests branch February 26, 2015 03:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: datastore Issues related to the Datastore API. cla: yes This human has signed the Contributor License Agreement. testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.