-
Notifications
You must be signed in to change notification settings - Fork 136
test: only use one backup for integration test #1581
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Restore operations cannot be cancelled, so we need to ignore any errors returned from cancelling the operation, and we need to wait for the restore operation to finish before we can delete the backup.
| boolean allDbOpsDone = false; | ||
| while (!allDbOpsDone) { | ||
| allDbOpsDone = true; | ||
| assertNotNull(backupMetadata.getProto()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be pushed outside of the while loop, since we only update the backupMetadata in the beginning of the try block?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, good point. Done.
| logger.info(String.format("Creating test database %s", db1Id)); | ||
| OperationFuture<Database, CreateDatabaseMetadata> dbOp1 = | ||
| logger.info(String.format("Creating test database %s", databaseId)); | ||
| OperationFuture<Database, CreateDatabaseMetadata> dbOp = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: dbOp -> databaseOperation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| // List all backups. | ||
| logger.info("Listing all backups"); | ||
| assertThat(instance.listBackups().iterateAll()).containsAtLeast(backup1, backup2); | ||
| assertThat(instance.listBackups().iterateAll()).contains(backup); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: replace assertThat by assertEquals, assertTrue, ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, here and elsewhere in this file
Changes the integration test for backups to only create one backup to reduce the probability of a build timeout, and increases the build timeout for the
SlowTestexecution to 7,200 seconds.Fixes #1436