test: enable tests to run on kokoro

[skuruppu]

No description provided.

[skuruppu]

@larkee or @IlyaFaer I'm trying to get the tests running on Kokoro. But as you can see above, the run fails with errors like:

____________________ ERROR at setup of test_get_table_names ____________________
Traceback (most recent call last):
  File "/tmpfs/src/github/python-spanner-sqlalchemy/.nox/snippets/lib/python3.8/site-packages/google/api_core/grpc_helpers.py", line 66, in error_remapped_callable
    return callable_(*args, **kwargs)
  File "/tmpfs/src/github/python-spanner-sqlalchemy/.nox/snippets/lib/python3.8/site-packages/grpc/_channel.py", line 946, in __call__
    return _end_unary_response_blocking(state, call, False, None)
  File "/tmpfs/src/github/python-spanner-sqlalchemy/.nox/snippets/lib/python3.8/site-packages/grpc/_channel.py", line 849, in _end_unary_response_blocking
    raise _InactiveRpcError(state)
grpc._channel._InactiveRpcError: <_InactiveRpcError of RPC that terminated with:
	status = StatusCode.NOT_FOUND
	details = "Instance not found: projects/precise-truck-742/instances/sqlalchemy-dialect-test"
	debug_error_string = "{"created":"@1634185605.444094995","description":"Error received from peer ipv4:74.125.142.95:443","file":"src/core/lib/surface/call.cc","file_line":1069,"grpc_message":"Instance not found: projects/precise-truck-742/instances/sqlalchemy-dialect-test","grpc_status":5}"

But you can see in the logs that nox > python create_test_database.py is executed without error. So I'm not quite sure why the tests are not able to find the instance.

@larkee I don't have permission on that project to see the instances.

Any help from either of you is appreciated. Then we can get rid of CircleCI which has been pretty flaky anyway.

[IlyaFaer]

@skuruppu, oh, I think I see where's the problem.

When we're running tests, we're actually creating a new instance and rewriting a config file, see:

python-spanner-sqlalchemy/create_test_database.py

Lines 75 to 79 in 1c3ff7d

	instance_id = (
	"sqlalchemy-dialect-test"
	if USE_EMULATOR
	else "sqlalchemy-test" + unique_resource_id
	)

But this PR forces tests to be executed on sqlalchemy-dialect-test. So, the created instance has another name, not sqlalchemy-dialect-test, but "sqlalchemy-test" + unique_resource_id

[skuruppu]

Ah thanks a lot @IlyaFaer. That explains it. So it was working fine for the migration tests which were reading test.cfg. But in a few other places, the code wasn't attempting to read the config file. The tests worked in the old project because we always had that static instance/database around but failed when we changed the project we were using. I've fixed them all now to read the url from the config file where possible.

[skuruppu]

@IlyaFaer would you please be able to take a look at the 3 snippets test failures here?

[IlyaFaer]

@skuruppu, that's quite strange. Some methods are using rowcount under the hood (but I don't see the case for now - ouside the snippets those methods are working fine). Speaking of which, they're using it like this:

Taking a closer look

[IlyaFaer]

Okay, I've investigated it further. Looks like we need some small changes to throw a warning instead of an exception, 'cause we can't omit those methods and we can't support rowcount.

I've also tried to set dialect requirements to say rowcount is not supported, but technically it doesn't fix this particular error. So, I guess I'll push one PR into the original client:
googleapis/python-spanner#628

And some not very meaningful (but still should have) changes here.

[IlyaFaer]

LGTM, we just need to wait for the other PR to be merged - it'll fix the failing tests

[IlyaFaer]

@skuruppu, looks like I can't restart Kokoro tests. Could you?

[skuruppu]

@IlyaFaer thanks a lot, the tests pass \o/

[IlyaFaer]

@skuruppu, I guess now we need to make ci/circleci: build-and-test not required in the repository settings (I'd do this, but I don't have rights). Then we'll be able to merge the PR.

[skuruppu]

Thanks @IlyaFaer. I should be able to fix the required check. I'm just waiting on @larkee to take a look as well since I wrote some very repetitive code here and if they have ideas on how to refactor out some of the code duplication then I wanted to fix that too before I merge it.

[larkee]

An argument could be made to make a base test class that sets up the engine and metadata that these other test classes inherit but I don't think it's necessary for this PR

[skuruppu]

Thanks for the review @larkee. PTAL

I addressed your feedback except for the one about creating a base class for setting up the engine. @IlyaFaer if this is something that you can do, then I'm happy for you to do so in a separate PR.

[IlyaFaer]

@skuruppu, okay, created an issue not to forget: #138
Will do.