Skip to content

Adding system test to exercise all possible Spanner types. - #3384

Merged
dhermes merged 2 commits into
googleapis:masterfrom
dhermes:fix-3016
May 9, 2017
Merged

Adding system test to exercise all possible Spanner types.#3384
dhermes merged 2 commits into
googleapis:masterfrom
dhermes:fix-3016

Conversation

@dhermes

Copy link
Copy Markdown
Contributor

In the process, changing how +/- Infinity are sent to the backend (this was a bug).

Fixes#3016.

/cc @vkedia It's worth noting that Value(string_value='inf') / Value(string_value='+inf') / Value(string_value='-inf') are being rejected, but should probably be accepted when Type.code == FLOAT64.

@dhermesdhermes added api: spanner Issues related to the Spanner API. testing labels May 8, 2017
@googlebotgooglebot added the cla: yes This human has signed the Contributor License Agreement. label May 8, 2017
@dhermes

Copy link
Copy Markdown
ContributorAuthor

Just created this branch on the GCP remote so that we'd get a build with system tests

@vkedia

Copy link
Copy Markdown

@dhermes It has to be "Infinity" and "-Infinity"

def _list_instances():
return list(Config.CLIENT.list_instances())

instances = retry(_list_instances)()

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

Comment threadspanner/tests/system.py Outdated
for found, expected in zip(row_data, self.ROW_DATA):
def _assert_timestamp(self, value, nano_value):
self.assertIsInstance(value, datetime.datetime)
self.assertIn(value.tzinfo, (None, UTC))

This comment was marked as spam.

This comment was marked as spam.

Comment threadspanner/tests/system.py Outdated
self.assertEqual(value.minute, nano_value.minute)
self.assertEqual(value.second, nano_value.second)
self.assertEqual(value.microsecond, nano_value.microsecond)
if isinstance(value, TimestampWithNanoseconds):

This comment was marked as spam.

This comment was marked as spam.

Comment threadspanner/tests/system.py Outdated
yield [index, 'First%09d' % (index,), 'Last09%d' % (index),
'test-%09d@example.com' % (index,)]

keyset = KeySet(all_=True)

This comment was marked as spam.

This comment was marked as spam.

@dhermes

Copy link
Copy Markdown
ContributorAuthor

It has to be "Infinity" and "-Infinity"

@vkedia I realize that now (made the necessary change in this PR). I'm suggesting that you also accept inf/ +inf / -inf.

@tseaver

Copy link
Copy Markdown
Contributor

@dhermes The principle you were trying to name in today's call is Postel's Law:

An implementation should be conservative in its sending behavior, and liberal in its receiving behavior

@dhermes

Copy link
Copy Markdown
ContributorAuthor

@tseaver We good to merge this?

@tseaver

Copy link
Copy Markdown
Contributor

Ugh, I screwed up merging mine first.

@dhermes

Copy link
Copy Markdown
ContributorAuthor

I'll fix it.

@googlebot

Copy link
Copy Markdown

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@googlebotgooglebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels May 9, 2017
@tseaver

Copy link
Copy Markdown
Contributor

I just tweaked it here. Feel free to back out my change.

Also used our GrpcRendezvous alias for _Rendezvous and making
some "conditionally defined" functions into a module-level function
and a test case staticmethod.
@tseaver

Copy link
Copy Markdown
Contributor

And merge when green.

In the process, changing how +/- Infinity are sent to the backend
(this was a bug).
Fixesgoogleapis#3016.
@googlebot

Copy link
Copy Markdown

CLAs look good, thanks!

@googlebotgooglebot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels May 9, 2017
@dhermes

Copy link
Copy Markdown
ContributorAuthor

Ah sorry, just did a force push before seeing your comment.

@tseaver

Copy link
Copy Markdown
Contributor

No worries.

@dhermes

Copy link
Copy Markdown
ContributorAuthor

Will wait on build on the GCP remote: https://circleci.com/gh/GoogleCloudPlatform/google-cloud-python/1785

@tseaver

Copy link
Copy Markdown
Contributor

Green on CircleCI.

@dhermes
dhermes merged commit 391c6fd into googleapis:masterMay 9, 2017
@dhermes
dhermes deleted the fix-3016 branch May 9, 2017 20:30
parthea pushed a commit that referenced this pull request Mar 9, 2026
* Spanner test hygiene: move fixtures out of distro.
Also used our GrpcRendezvous alias for _Rendezvous and making
some "conditionally defined" functions into a module-level function
and a test case staticmethod.
* Adding system test to exercise all possible Spanner types.
In the process, changing how +/- Infinity are sent to the backend
(this was a bug).
Fixes#3016.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: spannerIssues related to the Spanner API.cla: yesThis human has signed the Contributor License Agreement.testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@dhermes@vkedia@tseaver@googlebot@lukesneeringer