chore(tests): add conformance tests to CI for v3 #870
Conversation
|
🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use -- conventional-commit-lint bot |
product-auto-label
bot
added
the
api: bigtable
daniel-sanche
changed the title
| runs-on: ubuntu-latest | ||
| strategy: | ||
| matrix: | ||
| py-version: [ 3.8 ] |
There was a problem hiding this comment.
Python's case is a bit complicated, since we have 2 (and soon 3) different clients we need to test. I thought it would make sense to just test the lowest Python version for now, to prevent the number of tests exploding too much. But we can easily add more in the future
| popd | ||
|
|
||
| # Run the conformance test | ||
| pushd cloud-bigtable-clients-test/tests |
There was a problem hiding this comment.
Same comment as you made on my PR (googleapis/nodejs-bigtable#1349 (comment)). The push and pop are not necessary if you stay at the root.
There was a problem hiding this comment.
My comment was that pushd . is unnecessary if you aren't planning on changing directories
In this case, I am using pushd to move to the ./cloud-bigtable-clients-test/tests folder for the next commands, before popping back to the root
.kokoro/conformance.sh
Outdated
| popd | ||
|
|
||
| # Stop the proxy | ||
| kill $proxyPID |
There was a problem hiding this comment.
Same comment Baha made here: googleapis/nodejs-bigtable#1349 (comment). Maybe we could do this in a trap so that the process always exits.
There was a problem hiding this comment.
Good catch, fixed
| self._emulator_host = os.getenv(BIGTABLE_EMULATOR) | ||
| if self._emulator_host is not None and credentials is None: | ||
| # use insecure channel if emulator is set | ||
| credentials = google.auth.credentials.AnonymousCredentials() |
There was a problem hiding this comment.
How are the credentials related to adding conformance tests?
There was a problem hiding this comment.
Because this is the first time they are being run in a CI environment without GCP authentication, which was raising an error even in emulator mode
There was a problem hiding this comment.
Looks good to me. It seems like this situation is a lot like something I had to do in datastore for setting insecure credentials when the emulator is being used. See https://github.com/googleapis/nodejs-datastore/pull/1164/files#diff-a2a171449d862fe29692ce031981047d7ab755ae7f84c707aef80701b3ea0c80R516. I guess this is a bug fix as well then?
I might recommend in a future PR to also use the insecure credentials if an endpoint is provided in the constructor to hit the emulator when this environment variable is not provided.
| @@ -79,54 +79,157 @@ async def ReadRows(self, request, **kwargs): | |||
| row_list.append(dict_val) | |||
| return row_list | |||
|
|
|||
| @client_handler.error_safe | |||
There was a problem hiding this comment.
Are changes in this file necessary for the conformance tests to function?
There was a problem hiding this comment.
Yes, the conformance tests run through the test proxy. This is filling in some gaps in the test proxy implementation for the legacy client
There was a problem hiding this comment.
Sounds good. Changes here are pretty low risk since they don't affect client behavior so I guess they can be worked on over time until all test cases are handled.
|
LGTM |
* feat: add new v3.0.0 API skeleton (#745) * feat: improve rows filters (#751) * feat: read rows query model class (#752) * feat: implement row and cell model classes (#753) * feat: add pooled grpc transport (#748) * feat: implement read_rows (#762) * feat: implement mutate rows (#769) * feat: literal value filter (#767) * feat: row_exists and read_row (#778) * feat: read_modify_write and check_and_mutate_row (#780) * feat: sharded read rows (#766) * feat: ping and warm with metadata (#810) * feat: mutate rows batching (#770) * chore: restructure module paths (#816) * feat: improve timeout structure (#819) * fix: api errors apply to all bulk mutations * chore: reduce public api surface (#820) * feat: improve error group tracebacks on < py11 (#825) * feat: optimize read_rows (#852) * chore: add user agent suffix (#842) * feat: optimize retries (#854) * feat: add test proxy (#836) * chore(tests): add conformance tests to CI for v3 (#870) * chore(tests): turn off fast fail for conformance tets (#882) * feat: add TABLE_DEFAULTS enum for table method arguments (#880) * fix: pass None for retry in gapic calls (#881) * feat: replace internal dictionaries with protos in gapic calls (#875) * chore: optimize gapic calls (#863) * feat: expose retryable error codes to users (#879) * chore: update api_core submodule (#897) * chore: merge main into experimental_v3 (#900) * chore: pin conformance tests to v0.0.2 (#903) * fix: bulk mutation eventual success (#909) --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
adds config to run conformance tests pre-submit