Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding datastore _DEFAULTS stubs in all tests that use it. #665

Merged
merged 1 commit into from
Feb 19, 2015

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Feb 18, 2015

NOTE: Has #664 as diffbase

Introduces

  • concept of implicit for _DefaultsContainer.
  • testing module for datastore for patching

Verified all test cases that would accidentally rely on _DEFAULTS implicit
behavior by using a custom _DefaultsContainer that raises when the
instance is implicit.

Main delta for the custom properties:

diff --git a/gcloud/datastore/_implicit_environ.py b/gcloud/datastore/_implicit_environ.py
index 3afc954..1d38562 100644
--- a/gcloud/datastore/_implicit_environ.py
+++ b/gcloud/datastore/_implicit_environ.py
@@ -38,9 +38,37 @@ class _DefaultsContainer(object):
 :param dataset_id: Persistent implied dataset ID from environment.
 """

-    def __init__(self, connection=None, dataset_id=None):
-        self.connection = connection
-        self.dataset_id = dataset_id
+    class FooError(Exception):
+        pass
+
+    @property
+    def dataset_id(self):
+        if self.implicit:
+            raise self.FooError
+        return self._dataset_id
+
+    @dataset_id.setter
+    def dataset_id(self, value):
+        if self.implicit:
+            raise self.FooError
+        self._dataset_id = value
+
+    @property
+    def connection(self):
+        if self.implicit:
+            raise self.FooError
+        return self._connection
+
+    @connection.setter
+    def connection(self, value):
+        if self.implicit:
+            raise self.FooError
+        self._connection = value
+
+    def __init__(self, connection=None, dataset_id=None, implicit=False):
+        self.implicit = implicit
+        self._connection = connection
+        self._dataset_id = dataset_id

 def app_engine_id():

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 18, 2015
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 1f990e0 on dhermes:lazy-loading-attempt-2 into 72b6359 on GoogleCloudPlatform:master.

@dhermes dhermes added the api: datastore Issues related to the Datastore API. label Feb 19, 2015
@dhermes dhermes mentioned this pull request Feb 19, 2015
"""

def __init__(self, connection=None, dataset_id=None, implicit=False):
self.implicit = implicit

This comment was marked as spam.

@tseaver
Copy link
Contributor

tseaver commented Feb 19, 2015

I don't grasp the "main delta" bit: are you saying that was stuff you used to find places relying on implicit values, but didn't check it in after fixing them?

@dhermes
Copy link
Contributor Author

dhermes commented Feb 19, 2015

This PR is really just about using _monkey_defaults, _setup_defaults and _tear_down_defaults in tests.

The diff above is a hack I used to sniff out tests that relied on implicit behavior. It makes it so that anytime connection or dataset_id are get or set on a _DefaultsContainer, the following occurs:

        if self.implicit:
            raise self.FooError

So if the container being tested was implicit, the test would fail. (I made a custom error, so it didn't get caught by any assertRaises)

Introduces
- concept of `implicit` for `_DefaultsContainer`.
- testing module for datastore for patching

Verified all test cases that would accidentally rely on _DEFAULTS implicit
behavior by using a custom `_DefaultsContainer` that raises when the
instance is implicit.

Main delta for the custom properties:

diff --git a/gcloud/datastore/_implicit_environ.py b/gcloud/datastore/_implicit_environ.py
index 3afc954..1d38562 100644
--- a/gcloud/datastore/_implicit_environ.py
+++ b/gcloud/datastore/_implicit_environ.py
@@ -38,9 +38,37 @@ class _DefaultsContainer(object):
     :param dataset_id: Persistent implied dataset ID from environment.
     """

-    def __init__(self, connection=None, dataset_id=None):
-        self.connection = connection
-        self.dataset_id = dataset_id
+    class FooError(Exception):
+        pass
+
+    @Property
+    def dataset_id(self):
+        if self.implicit:
+            raise self.FooError
+        return self._dataset_id
+
+    @dataset_id.setter
+    def dataset_id(self, value):
+        if self.implicit:
+            raise self.FooError
+        self._dataset_id = value
+
+    @Property
+    def connection(self):
+        if self.implicit:
+            raise self.FooError
+        return self._connection
+
+    @connection.setter
+    def connection(self, value):
+        if self.implicit:
+            raise self.FooError
+        self._connection = value
+
+    def __init__(self, connection=None, dataset_id=None, implicit=False):
+        self.implicit = implicit
+        self._connection = connection
+        self._dataset_id = dataset_id

 def app_engine_id():
@dhermes dhermes force-pushed the lazy-loading-attempt-2 branch from 1f990e0 to 5e281dd Compare February 19, 2015 19:10
@dhermes
Copy link
Contributor Author

dhermes commented Feb 19, 2015

@tseaver I just rebased on top of #664.

Remaining issues here?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 5e281dd on dhermes:lazy-loading-attempt-2 into 998c724 on GoogleCloudPlatform:master.

@tseaver
Copy link
Contributor

tseaver commented Feb 19, 2015

LGTM. (I think some of this commit gets backed out in #667 anyway).

dhermes added a commit that referenced this pull request Feb 19, 2015
Adding datastore _DEFAULTS stubs in all tests that use it.
@dhermes dhermes merged commit 6ff634a into googleapis:master Feb 19, 2015
@dhermes dhermes deleted the lazy-loading-attempt-2 branch February 19, 2015 19:29
vchudnov-g pushed a commit that referenced this pull request Sep 20, 2023
* feat: added baseline model version used to generate the summary
feat: added the platform of the virtual agent response messages

PiperOrigin-RevId: 555788605

Source-Link: googleapis/googleapis@90a551c

Source-Link: googleapis/googleapis-gen@30620b4
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiMzA2MjBiNGYzNDcxMzg2MDA4M2ZmZjAzNjk4MjkwMGIxMDA0Y2JmNSJ9

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

---------

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants