Skip to content

Commit

Permalink
Refactor conformance tests. (#6291)
Browse files Browse the repository at this point in the history
Closes #6290.

Breaking change from `firestore-0.30.0`: revert to merge not being an option;
instead make it a bool or a list param to `set`.

Use 'pytest.mark.parametrize' to create a testcase per textproto file.

Blacklist conformance tests for transforms we don't yet have (ArrayDelete, ArrayUnion, and Delete)

Re-import google-cloud-common testdata textprotos:  discard older, renamed versions.
  • Loading branch information
tseaver committed Nov 15, 2018
1 parent 5c4b1d3 commit 4d29c1f
Show file tree
Hide file tree
Showing 107 changed files with 1,663 additions and 2,988 deletions.
470 changes: 394 additions & 76 deletions google/cloud/firestore_v1beta1/_helpers.py

Large diffs are not rendered by default.

18 changes: 13 additions & 5 deletions google/cloud/firestore_v1beta1/batch.py
Expand Up @@ -57,8 +57,8 @@ def create(self, reference, document_data):
document_data (dict): Property names and values to use for
creating a document.
"""
write_pbs = _helpers.pbs_for_set(
reference._document_path, document_data, merge=False, exists=False)
write_pbs = _helpers.pbs_for_create(
reference._document_path, document_data)
self._add_write_pbs(write_pbs)

def set(self, reference, document_data, merge=False):
Expand All @@ -74,12 +74,17 @@ def set(self, reference, document_data, merge=False):
A document reference that will have values set in this batch.
document_data (dict):
Property names and values to use for replacing a document.
merge (Optional[bool]):
merge (Optional[bool] or Optional[List<apispec>]):
If True, apply merging instead of overwriting the state
of the document.
"""
write_pbs = _helpers.pbs_for_set(
reference._document_path, document_data, merge=merge)
if merge is not False:
write_pbs = _helpers.pbs_for_set_with_merge(
reference._document_path, document_data, merge)
else:
write_pbs = _helpers.pbs_for_set_no_merge(
reference._document_path, document_data)

self._add_write_pbs(write_pbs)

def update(self, reference, field_updates, option=None):
Expand All @@ -98,6 +103,9 @@ def update(self, reference, field_updates, option=None):
write option to make assertions / preconditions on the server
state of the document before applying changes.
"""
if option.__class__.__name__ == 'ExistsOption':
raise ValueError('you must not pass an explicit write option to '
'update.')
write_pbs = _helpers.pbs_for_update(
self._client, reference._document_path, field_updates, option)
self._add_write_pbs(write_pbs)
Expand Down
5 changes: 3 additions & 2 deletions google/cloud/firestore_v1beta1/client.py
Expand Up @@ -23,7 +23,6 @@
* a :class:`~.firestore_v1beta1.client.Client` owns a
:class:`~.firestore_v1beta1.document.DocumentReference`
"""

from google.cloud.client import ClientWithProject

from google.cloud.firestore_v1beta1 import _helpers
Expand All @@ -39,7 +38,9 @@
DEFAULT_DATABASE = '(default)'
"""str: The default database used in a :class:`~.firestore.client.Client`."""
_BAD_OPTION_ERR = (
'Exactly one of ``last_update_time`` or ``exists`` must be provided.')
'Exactly one of ``last_update_time`` or ``exists`` '
'must be provided.'
)
_BAD_DOC_TEMPLATE = (
'Document {!r} appeared in response but was not present among references')
_ACTIVE_TXN = 'There is already an active transaction.'
Expand Down
6 changes: 3 additions & 3 deletions google/cloud/firestore_v1beta1/document.py
Expand Up @@ -211,9 +211,9 @@ def set(self, document_data, merge=False):
Args:
document_data (dict): Property names and values to use for
replacing a document.
option (Optional[~.firestore_v1beta1.client.WriteOption]): A
write option to make assertions / preconditions on the server
state of the document before applying changes.
merge (Optional[bool] or Optional[List<apispec>]):
If True, apply merging instead of overwriting the state
of the document.
Returns:
google.cloud.firestore_v1beta1.types.WriteResult: The
Expand Down
9 changes: 3 additions & 6 deletions tests/system.py
Expand Up @@ -202,8 +202,7 @@ def test_document_integer_field(client, cleanup):
document.create(data1)

data2 = {'1a.ab': '4d', '6f.7g': '9h'}
option2 = client.write_option(exists=True)
document.update(data2, option=option2)
document.update(data2)
snapshot = document.get()
expected = {
'1a': {
Expand Down Expand Up @@ -311,9 +310,8 @@ def test_update_document(client, cleanup):
assert document_id in exc_info.value.message

# 1. Try to update before the document exists (now with an option).
option1 = client.write_option(exists=True)
with pytest.raises(NotFound) as exc_info:
document.update({'still': 'not-there'}, option=option1)
document.update({'still': 'not-there'})
assert exc_info.value.message.startswith(MISSING_DOCUMENT)
assert document_id in exc_info.value.message

Expand All @@ -327,8 +325,7 @@ def test_update_document(client, cleanup):
},
'other': True,
}
option2 = client.write_option(exists=False)
write_result2 = document.update(data, option=option2)
write_result2 = document.create(data)

# 3. Send an update without a field path (no option).
field_updates3 = {'foo': {'quux': 800}}
Expand Down

0 comments on commit 4d29c1f

Please sign in to comment.