feat(spanner): tests and samples for drop-database protection #11637
Conversation
Exercise the new `DatabaseAdminClient::UpdateDatabase()` interface to set the `Database.enable_drop_protection` field.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #11637 +/- ##
=======================================
Coverage ? 93.77%
=======================================
Files ? 1824
Lines ? 164416
Branches ? 0
=======================================
Hits ? 154184
Misses ? 10232
Partials ? 0
☔ View full report in Codecov by Sentry. |
|
Here is the summary of changes. You are about to add 1 region tag.
This comment is generated by snippet-bot.
|
| @@ -581,6 +581,69 @@ TEST_F(DatabaseAdminClientTest, DatabasePostgreSQLBasics) { | |||
| EXPECT_STATUS_OK(drop_status); | |||
| } | |||
|
|
|||
| /// @test Verify dropping a database fails with drop protection enabled. | |||
| TEST_F(DatabaseAdminClientTest, DropDatabaseProtection) { | |||
There was a problem hiding this comment.
Is this a test of our code or a test of the service? I do not think we want to spend a lot of time testing the service, that is not our job.
There was a problem hiding this comment.
I'm (almost) totally with you. It does exercise our generated code a little, but the overarching reason for it is to cover a "thou shalt have" clause in the feature-specification doc that isn't worth contesting. :-|
There was a problem hiding this comment.
I think it is worth contesting, but not blocking this PR for it.
| database->set_enable_drop_protection(true); | ||
| auto updated = client_.UpdateDatabase(*database, update_mask).get(); | ||
| if (emulator_) { | ||
| EXPECT_THAT(updated, StatusIs(StatusCode::kUnimplemented)); |
There was a problem hiding this comment.
Consider just skipping the test in this case. We already have tests for CreateDatabase() and DropDatabase().
There was a problem hiding this comment.
The idea there is to get some active notification of when the behavior of the emulator changes.
We do have some instances of if (emulator) GTEST_SKIP(), but they are either old, or where the above tactic is too complicated.
| } | ||
| google::cloud::spanner_admin::DatabaseAdminClient client( | ||
| google::cloud::spanner_admin::MakeDatabaseAdminConnection()); | ||
| bool drop_protection = (argv[3][0] == 'T'); |
There was a problem hiding this comment.
assuming ! argv[3].empty() ?
There was a problem hiding this comment.
s[i] is actually [and weirdly] defined to return (a reference to) a char() value when i == s.size().
Exercise the new
DatabaseAdminClient::UpdateDatabase()interface to set theDatabase.enable_drop_protectionfield.This change is