diff --git a/handwritten/bigtable/src/client-side-metrics/exporter.ts b/handwritten/bigtable/src/client-side-metrics/exporter.ts index bfacf014951..9f8a3bca813 100644 --- a/handwritten/bigtable/src/client-side-metrics/exporter.ts +++ b/handwritten/bigtable/src/client-side-metrics/exporter.ts @@ -53,9 +53,13 @@ function getInterval( const startSec = dataPoint.startTime[0]; const startNanos = dataPoint.startTime[1]; - if (startSec > endSec || (startSec === endSec && startNanos > endNanos)) { + if (startSec > endSec || (startSec === endSec && startNanos >= endNanos)) { endSec = startSec; - endNanos = startNanos; + endNanos = startNanos + 1_000_000; + if (endNanos >= 1_000_000_000) { + endSec += 1; + endNanos -= 1_000_000_000; + } } return { diff --git a/handwritten/bigtable/src/utils/mutateInternal.ts b/handwritten/bigtable/src/utils/mutateInternal.ts index 1e084b92a79..146f5527dba 100644 --- a/handwritten/bigtable/src/utils/mutateInternal.ts +++ b/handwritten/bigtable/src/utils/mutateInternal.ts @@ -12,16 +12,13 @@ // See the License for the specific language governing permissions and // limitations under the License. -import { - Entry, - MutateCallback, - MutateOptions, - PartialFailureError, -} from '../table'; +import {Entry, MutateOptions} from '../table'; import {OperationMetricsCollector} from '../client-side-metrics/operation-metrics-collector'; import { DEFAULT_BACKOFF_SETTINGS, getNextDelay, + MutateCallback, + PartialFailureError, populateAttemptHeader, RETRYABLE_STATUS_CODES, TabularApiSurface, diff --git a/handwritten/bigtable/system-test/bigtable.ts b/handwritten/bigtable/system-test/bigtable.ts index 1550e8e1c65..fc1b32f89a9 100644 --- a/handwritten/bigtable/system-test/bigtable.ts +++ b/handwritten/bigtable/system-test/bigtable.ts @@ -44,6 +44,8 @@ import google = protos.google; type IBackup = google.bigtable.admin.v2.IBackup; +const BACKUP_QUOTA_COOLDOWN_MS = 60001; + describe('Bigtable', () => { const bigtable = new Bigtable(); const INSTANCE = bigtable.instance(generateId('instance')); @@ -1370,10 +1372,6 @@ describe('Bigtable', () => { // the real functionality - it is just a way to keep things organized! const backupIdFromCluster = generateId('backup'); let backupNameFromCluster: string; - const restoreTableIdFromCluster = generateId('table'); - - const backupIdFromTable = generateId('backup'); - let backupNameFromTable: string; // The minimum backup expiry time is 6 hours. The times here each have a 2 // hour padding to tolerate latency and clock drift. Also, while the time @@ -1385,20 +1383,18 @@ describe('Bigtable', () => { ); before(async () => { - const [backup, op] = await CLUSTER.createBackup(backupIdFromCluster, { - table: TABLE, - expireTime, - }); + const [backup, op] = await withBackupQuotaRetry(() => + CLUSTER.createBackup(backupIdFromCluster, { + table: TABLE, + expireTime, + }), + ); BACKUP = backup; await op.promise(); backupNameFromCluster = replaceProjectIdToken( `${CLUSTER.name}/backups/${backupIdFromCluster}`, bigtable.projectId, ); - backupNameFromTable = replaceProjectIdToken( - `${CLUSTER.name}/backups/${backupIdFromTable}`, - bigtable.projectId, - ); }); it('should create backup of a table (from cluster)', async () => { @@ -1410,15 +1406,30 @@ describe('Bigtable', () => { }); it('should create backup of a table (from table)', async () => { - const [backup, op] = await TABLE.createBackup(backupIdFromTable, { - expireTime, - }); - await op.promise(); - await backup.getMetadata(); + const backupIdFromTable = generateId('backup'); + const backupNameFromTable = replaceProjectIdToken( + `${CLUSTER.name}/backups/${backupIdFromTable}`, + bigtable.projectId, + ); + const [backup, op] = await withBackupQuotaRetry(() => + TABLE.createBackup(backupIdFromTable, { + expireTime, + }), + ); + try { + await op.promise(); + await backup.getMetadata(); - assert.strictEqual(backup.metadata!.name, backupNameFromTable); + assert.strictEqual(backup.metadata!.name, backupNameFromTable); - assert.deepStrictEqual(backup.expireDate, expireTime); + assert.deepStrictEqual(backup.expireDate, expireTime); + } finally { + try { + await withBackupQuotaRetry(() => backup.delete()); + } catch { + // Ignore cleanup errors if the backup was not created. + } + } }); it('should get a specific backup (cluster)', async () => { @@ -1470,6 +1481,7 @@ describe('Bigtable', () => { }); it('should restore a backup (cluster)', async () => { + const restoreTableIdFromCluster = generateId('table'); const backup = CLUSTER.backup(backupIdFromCluster); const [table, op] = await backup.restore(restoreTableIdFromCluster); await op.promise(); @@ -1479,34 +1491,42 @@ describe('Bigtable', () => { }); it('should restore a backup to a different instance', async () => { - const [, operation] = await DIFF_INSTANCE.create( - createInstanceConfig( - generateId('d-clust'), - 'us-east1-c', - 3, - Date.now(), - ), - ); - await operation.promise(); + const [alreadyExists] = await DIFF_INSTANCE.exists(); + if (!alreadyExists) { + const [, operation] = await DIFF_INSTANCE.create( + createInstanceConfig( + generateId('d-clust'), + 'us-east1-c', + 3, + Date.now(), + ), + ); + await operation.promise(); + } const [iExists] = await DIFF_INSTANCE.exists(); assert.strictEqual(iExists, true); + const restoreTableIdFromCluster = generateId('table'); const backup = CLUSTER.backup(backupIdFromCluster); const [table, op] = await backup.restoreTo({ tableId: restoreTableIdFromCluster, instance: DIFF_INSTANCE, }); await op.promise(); - const [tExists] = await table.exists(); + const [tExists] = await DIFF_INSTANCE.table( + restoreTableIdFromCluster, + ).exists(); assert.strictEqual(tExists, true); assert.strictEqual(table.id, restoreTableIdFromCluster); }); it('should update a backup (cluster)', async () => { const backup = CLUSTER.backup(backupIdFromCluster); - const [metadata] = await backup.setMetadata({ - expireTime: updateExpireTime, - }); + const [metadata] = await withBackupQuotaRetry(() => + backup.setMetadata({ + expireTime: updateExpireTime, + }), + ); assert.strictEqual(metadata.name, backupNameFromCluster); assert.deepStrictEqual(backup.expireDate, updateExpireTime); @@ -1554,7 +1574,7 @@ describe('Bigtable', () => { // Sleep here for just over a minute so that the system tests don't // experience quota issues due to too many requests per minute. await new Promise(resolve => { - setTimeout(resolve, 60001); + setTimeout(resolve, BACKUP_QUOTA_COOLDOWN_MS); }); }); /* @@ -1570,7 +1590,9 @@ describe('Bigtable', () => { const [backupsBeforeCopy] = await instance.getBackups(); const backupIdsBeforeCopy = backupsBeforeCopy.map(backup => backup.id); // Copy the backup - const [newBackup, operation] = await backup.copy(config); + const [newBackup, operation] = await withBackupQuotaRetry(() => + backup.copy(config), + ); try { assert.strictEqual(config.id, newBackup.id); await operation.promise(); @@ -1604,7 +1626,13 @@ describe('Bigtable', () => { assert.strictEqual(fetchedNewBackup.name, backupPath); // Delete the copied backup } finally { - await config.cluster.backup(newBackup.id).delete(); + try { + await withBackupQuotaRetry(() => + config.cluster.backup(newBackup.id).delete(), + ); + } catch { + // Ignore cleanup errors if the copied backup was not created. + } } } @@ -1613,9 +1641,11 @@ describe('Bigtable', () => { sourceTestExpireTime: BackupTimestamp, copyTestExpireTime: BackupTimestamp, ) { - const [backup, op] = await TABLE.createBackup(generateId('backup'), { - expireTime: sourceTestExpireTime, - }); + const [backup, op] = await withBackupQuotaRetry(() => + TABLE.createBackup(generateId('backup'), { + expireTime: sourceTestExpireTime, + }), + ); try { { await op.promise(); @@ -1633,7 +1663,11 @@ describe('Bigtable', () => { INSTANCE, ); } finally { - await backup.delete(); + try { + await withBackupQuotaRetry(() => backup.delete()); + } catch { + // Ignore cleanup errors if the source backup was not created. + } } } it('should copy to the same cluster with precise date expiry times', async () => { @@ -1655,9 +1689,11 @@ describe('Bigtable', () => { }); }); it('should create backup of a table and copy it on another cluster of another instance', async () => { - const [backup, op] = await TABLE.createBackup(generateId('backup'), { - expireTime: sourceExpireTime, - }); + const [backup, op] = await withBackupQuotaRetry(() => + TABLE.createBackup(generateId('backup'), { + expireTime: sourceExpireTime, + }), + ); try { { await op.promise(); @@ -1668,45 +1704,57 @@ describe('Bigtable', () => { // Create another instance const instance = bigtable.instance(generateId('instance')); const destinationClusterId = generateId('cluster'); - { - // Create production instance with given options - const instanceOptions: InstanceOptions = { - clusters: [ - { - id: destinationClusterId, - nodes: 1, - location: 'us-central2-d', - storage: 'ssd', - }, - ], - labels: { - 'prod-label': 'prod-label', - time_created: String(Date.now()), + // Create production instance with given options + const instanceOptions: InstanceOptions = { + clusters: [ + { + id: destinationClusterId, + nodes: 1, + location: 'us-central2-d', + storage: 'ssd', }, - type: 'production', - }; - const [, operation] = await instance.create(instanceOptions); + ], + labels: { + 'prod-label': 'prod-label', + time_created: String(Date.now()), + }, + type: 'production', + }; + const [, operation] = await instance.create(instanceOptions); + try { await operation.promise(); + // Create the copy and test the copied backup + await testCopyBackup( + backup, + { + cluster: new Cluster(instance, destinationClusterId), + id: generateId('backup'), + expireTime: copyExpireTime, + }, + instance, + ); + } finally { + await reapBackups(instance); + try { + await instance.delete(); + } catch { + // Ignore cleanup errors if the instance was not created. + } } - // Create the copy and test the copied backup - await testCopyBackup( - backup, - { - cluster: new Cluster(instance, destinationClusterId), - id: generateId('backup'), - expireTime: copyExpireTime, - }, - instance, - ); - await instance.delete(); } finally { - await backup.delete(); + try { + await withBackupQuotaRetry(() => backup.delete()); + } catch { + // Ignore cleanup errors if the source backup was not created. + } } }); it('should create backup of a table and copy it on another cluster of the same instance', async () => { - const [backup, op] = await TABLE.createBackup(generateId('backup'), { - expireTime: sourceExpireTime, - }); + const [backup, op] = await withBackupQuotaRetry(() => + TABLE.createBackup(generateId('backup'), { + expireTime: sourceExpireTime, + }), + ); try { { await op.promise(); @@ -1715,34 +1763,45 @@ describe('Bigtable', () => { assert.deepStrictEqual(backup.expireDate, sourceExpireTime); } const destinationClusterId = generateId('cluster'); - { - // Create destination cluster with given options - const [, operation] = await INSTANCE.cluster( - destinationClusterId, - ).create({ - location: 'us-central2-b', - nodes: 1, - }); + const destinationCluster = INSTANCE.cluster(destinationClusterId); + // Create destination cluster with given options + const [, operation] = await destinationCluster.create({ + location: 'us-central2-b', + nodes: 1, + }); + try { await operation.promise(); + // Create the copy and test the copied backup + await testCopyBackup( + backup, + { + cluster: new Cluster(INSTANCE, destinationClusterId), + id: generateId('backup'), + expireTime: copyExpireTime, + }, + INSTANCE, + ); + } finally { + try { + await destinationCluster.delete(); + } catch { + // Ignore cleanup errors if the destination cluster was not created. + } } - // Create the copy and test the copied backup - await testCopyBackup( - backup, - { - cluster: new Cluster(INSTANCE, destinationClusterId), - id: generateId('backup'), - expireTime: copyExpireTime, - }, - INSTANCE, - ); } finally { - await backup.delete(); + try { + await withBackupQuotaRetry(() => backup.delete()); + } catch { + // Ignore cleanup errors if the source backup was not created. + } } }); it('should create backup of a table and copy it on another project', async () => { - const [backup, op] = await TABLE.createBackup(generateId('backup'), { - expireTime: sourceExpireTime, - }); + const [backup, op] = await withBackupQuotaRetry(() => + TABLE.createBackup(generateId('backup'), { + expireTime: sourceExpireTime, + }), + ); try { { await op.promise(); @@ -1760,42 +1819,56 @@ describe('Bigtable', () => { generateId('instance'), ); const destinationClusterId = generateId('cluster'); - { - // Create production instance with given options - const instanceOptions: InstanceOptions = { - clusters: [ - { - id: destinationClusterId, - nodes: 1, - location: 'us-central2-d', - storage: 'ssd', - }, - ], - labels: {'prod-label': 'prod-label'}, - type: 'production', - }; - const [, operation] = await secondInstance.create(instanceOptions); + // Create production instance with given options + const instanceOptions: InstanceOptions = { + clusters: [ + { + id: destinationClusterId, + nodes: 1, + location: 'us-central2-d', + storage: 'ssd', + }, + ], + labels: {'prod-label': 'prod-label'}, + type: 'production', + }; + const [, operation] = await secondInstance.create(instanceOptions); + try { await operation.promise(); + // Create the copy and test the copied backup + await testCopyBackup( + backup, + { + cluster: new Cluster(secondInstance, destinationClusterId), + id: generateId('backup'), + expireTime: copyExpireTime, + }, + secondInstance, + ); + } finally { + await reapBackups(secondInstance); + try { + await secondInstance.delete(); + } catch { + // Ignore cleanup errors if the second instance was not created. + } } - // Create the copy and test the copied backup - await testCopyBackup( - backup, - { - cluster: new Cluster(secondInstance, destinationClusterId), - id: generateId('backup'), - expireTime: copyExpireTime, - }, - secondInstance, - ); - await secondInstance.delete(); } finally { - await backup.delete(); + try { + await withBackupQuotaRetry(() => backup.delete()); + } catch { + // Ignore cleanup errors if the source backup was not created. + } } }); it('should restore a copied backup', async () => { const backupId = generateId('backup'); - const table = INSTANCE.table('old-table'); - { + const table = INSTANCE.table(generateId('table')); + const restoredTableId = generateId('table'); + let backup: Backup | undefined; + let newBackup: Backup | undefined; + let newTable: Table | undefined; + try { // Create a table and insert data into it. await table.create(); await table.createFamily('follows'); @@ -1809,32 +1882,63 @@ describe('Bigtable', () => { }, }, ]); - } - // Create the backup - const [backup, createBackupOperation] = await table.createBackup( - backupId, - { - expireTime: sourceExpireTime, - }, - ); - try { + await table.waitForReplication(); + // Create the backup + const [createdBackup, createBackupOperation] = + await withBackupQuotaRetry(() => + table.createBackup(backupId, { + expireTime: sourceExpireTime, + }), + ); + backup = createdBackup; await createBackupOperation.promise(); // Copy the backup const config = { - cluster: backup.cluster, + cluster: createdBackup.cluster, id: generateId('backup'), expireTime: copyExpireTime, }; - const [newBackup, copyOperation] = await backup.copy(config); + const [copiedBackup, copyOperation] = await withBackupQuotaRetry(() => + createdBackup.copy(config), + ); + newBackup = copiedBackup; await copyOperation.promise(); // Restore a table from the copied backup - const [newTable, restoreOperation] = - await newBackup.restore('new-table'); + const [restoredTable, restoreOperation] = + await copiedBackup.restore(restoredTableId); + newTable = restoredTable; await restoreOperation.promise(); - const rows = await newTable.getRows(); + const rows = await restoredTable.getRows(); assert.deepStrictEqual(rows[0][0].id, 'some-data-to-copy-key'); } finally { - await backup.delete(); + if (newBackup) { + const copiedBackupToDelete = newBackup; + try { + await withBackupQuotaRetry(() => copiedBackupToDelete.delete()); + } catch { + // Ignore cleanup errors if the copied backup was not created. + } + } + if (backup) { + const sourceBackupToDelete = backup; + try { + await withBackupQuotaRetry(() => sourceBackupToDelete.delete()); + } catch { + // Ignore cleanup errors if the source backup was not created. + } + } + if (newTable) { + try { + await newTable.delete(); + } catch { + // Ignore cleanup errors if the restored table was not created. + } + } + try { + await table.delete(); + } catch { + // Ignore cleanup errors if the source table was not created. + } } }); }); @@ -1848,9 +1952,6 @@ describe('Bigtable', () => { let backupSSD: IBackup, backupHDD: IBackup; - const targetTableIdSSD = generateId('ttbl'), - targetTableIdHDD = generateId('ttbl'); - // We just test from table here. const backupIdFromTableSSD = generateId('backup'), backupIdFromTableHDD = generateId('backup'); @@ -1867,35 +1968,47 @@ describe('Bigtable', () => { beforeEach(async () => { // This is not ideal, but we are running into quota issues for admin API access. - await new Promise(r => setTimeout(r, 60 * 1000)); + await new Promise(resolve => + setTimeout(resolve, BACKUP_QUOTA_COOLDOWN_MS), + ); }); before(async () => { - const [backupOpSSD] = await tableAdmin.createBackup({ - parent: replaceProjectIdToken( - INSTANCE.cluster(CLUSTER_ID).name, - bigtable.projectId, - ), - backupId: backupIdFromTableSSD, - backup: { - expireTime, - sourceTable: replaceProjectIdToken(TABLE.name, bigtable.projectId), - }, - }); - const [backupOpHDD] = await tableAdmin.createBackup({ - parent: replaceProjectIdToken( - INSTANCE_HDD.cluster(CLUSTER_ID_HDD).name, - bigtable.projectId, - ), - backupId: backupIdFromTableHDD, - backup: { - expireTime, - sourceTable: replaceProjectIdToken( - TABLE_HDD.name, + // Sleep here for just over a minute before the "before all" hook creates + // backups so that requests from the preceding backup tests do not trigger + // per-minute 'Backup metadata write requests' quota exhaustion. + await new Promise(resolve => + setTimeout(resolve, BACKUP_QUOTA_COOLDOWN_MS), + ); + const [backupOpSSD] = await withBackupQuotaRetry(() => + tableAdmin.createBackup({ + parent: replaceProjectIdToken( + INSTANCE.cluster(CLUSTER_ID).name, bigtable.projectId, ), - }, - }); + backupId: backupIdFromTableSSD, + backup: { + expireTime, + sourceTable: replaceProjectIdToken(TABLE.name, bigtable.projectId), + }, + }), + ); + const [backupOpHDD] = await withBackupQuotaRetry(() => + tableAdmin.createBackup({ + parent: replaceProjectIdToken( + INSTANCE_HDD.cluster(CLUSTER_ID_HDD).name, + bigtable.projectId, + ), + backupId: backupIdFromTableHDD, + backup: { + expireTime, + sourceTable: replaceProjectIdToken( + TABLE_HDD.name, + bigtable.projectId, + ), + }, + }), + ); const [resSSD, resHDD] = await Promise.all([ backupOpSSD.promise(), backupOpHDD.promise(), @@ -1932,6 +2045,7 @@ describe('Bigtable', () => { it('should not optimize when restoring from the same backing type', async () => { // HDD -> HDD and SSD -> SSD should both not trigger an optimize pass. + const targetTableIdHDD = generateId('ttbl'); const [restoreOp] = await tableAdmin.restoreTable({ parent: replaceProjectIdToken(INSTANCE_HDD.name, bigtable.projectId), tableId: targetTableIdHDD, @@ -1943,6 +2057,7 @@ describe('Bigtable', () => { it('should optimize when restoring from a different backing type', async () => { // HDD -> SSD should trigger an optimize. + const targetTableIdSSD = generateId('ttbl'); const [restoreOp] = await tableAdmin.restoreTable({ parent: replaceProjectIdToken(INSTANCE.name, bigtable.projectId), tableId: targetTableIdSSD, @@ -2624,3 +2739,28 @@ function createInstanceConfig( }, }; } + +/** + * Executes a backup metadata write operation and retries after a 1-minute + * cooldown when a `RESOURCE_EXHAUSTED` (gRPC code 8) quota error is returned. + */ +async function withBackupQuotaRetry( + fn: () => Promise, + retries = 2, +): Promise { + for (let attempt = 0; ; attempt++) { + try { + return await fn(); + } catch (err) { + const isQuotaError = + (err as {code?: number})?.code === 8 || + (err as Error)?.message?.includes('RESOURCE_EXHAUSTED'); + if (!isQuotaError || attempt >= retries) { + throw err; + } + await new Promise(resolve => + setTimeout(resolve, BACKUP_QUOTA_COOLDOWN_MS), + ); + } + } +} diff --git a/handwritten/bigtable/system-test/client-side-metrics-all-methods.ts b/handwritten/bigtable/system-test/client-side-metrics-all-methods.ts index d0d8d9ebe49..569c236f666 100644 --- a/handwritten/bigtable/system-test/client-side-metrics-all-methods.ts +++ b/handwritten/bigtable/system-test/client-side-metrics-all-methods.ts @@ -357,7 +357,7 @@ async function checkForPublishedMetrics(projectId: string) { seriesLength = series.length; break; } - await new Promise(r => setTimeout(r, 2000)); + await new Promise(resolve => setTimeout(resolve, 2000)); } assert(seriesLength > 0); } @@ -482,10 +482,10 @@ describe('Bigtable/ClientSideMetricsAllMethods', () => { metrics: ResourceMetrics, resultCallback: (result: ExportResult) => void, ): Promise { - try { - await super.export(metrics, (result: ExportResult) => { - if (!exported) { - exported = true; + if (!exported) { + exported = true; + try { + await super.export(metrics, (result: ExportResult) => { try { clearTimeout(timeout); // The test passes when the code is 0 because that means the @@ -493,6 +493,7 @@ describe('Bigtable/ClientSideMetricsAllMethods', () => { assert.strictEqual(result.code, 0); resultCallback({code: 0}); void checkForPublishedMetrics(projectId) + // eslint-disable-next-line promise/always-return .then(() => { done(); }) @@ -503,12 +504,12 @@ describe('Bigtable/ClientSideMetricsAllMethods', () => { // The code here isn't 0 so we report the original error to the mocha test runner. done(error); } - } else { - resultCallback({code: 0}); - } - }); - } catch (error) { - done(error); + }); + } catch (error) { + done(error); + } + } else { + resultCallback({code: 0}); } } } @@ -1025,6 +1026,7 @@ describe('Bigtable/ClientSideMetricsAllMethods', () => { done: mocha.Done, onExportSuccess?: () => void, ) { + let exported = false; class TestExporter extends CloudMonitoringExporter { constructor(options: ClientOptions) { super(options); @@ -1034,28 +1036,32 @@ describe('Bigtable/ClientSideMetricsAllMethods', () => { metrics: ResourceMetrics, resultCallback: (result: ExportResult) => void, ): Promise { - try { - await super.export(metrics, async (result: ExportResult) => { - try { - // The code is expected to be 0 because the - // result from calling export was successful. - assert.strictEqual(result.code, 0); - resultCallback({code: 0}); - if (onExportSuccess) { - onExportSuccess(); + if (!exported) { + exported = true; + try { + await super.export(metrics, async (result: ExportResult) => { + try { + // The code is expected to be 0 because the + // result from calling export was successful. + assert.strictEqual(result.code, 0); + resultCallback({code: 0}); + if (onExportSuccess) { + onExportSuccess(); + } + } catch { + // The code here isn't 0 so we report the original error to the + // mocha test runner. + // The test fails here because it means that an export was + // unsuccessful. + done(result); + resultCallback({code: 0}); } - } catch (error) { - // The code here isn't 0 so we report the original error to the - // mocha test runner. - // The test fails here because it means that an export was - // unsuccessful. - done(result); - done(error); - resultCallback({code: 0}); - } - }); - } catch (error) { - done(error); + }); + } catch (error) { + done(error); + resultCallback({code: 0}); + } + } else { resultCallback({code: 0}); } } diff --git a/handwritten/bigtable/system-test/client-side-metrics.ts b/handwritten/bigtable/system-test/client-side-metrics.ts index 7a029b37378..35bd3f15740 100644 --- a/handwritten/bigtable/system-test/client-side-metrics.ts +++ b/handwritten/bigtable/system-test/client-side-metrics.ts @@ -334,7 +334,7 @@ async function checkForPublishedMetrics(projectId: string) { seriesLength = series.length; break; } - await new Promise(r => setTimeout(r, 2000)); + await new Promise(resolve => setTimeout(resolve, 2000)); } assert(seriesLength > 0); } @@ -459,10 +459,10 @@ describe('Bigtable/ClientSideMetrics', () => { metrics: ResourceMetrics, resultCallback: (result: ExportResult) => void, ): Promise { - try { - await super.export(metrics, (result: ExportResult) => { - if (!exported) { - exported = true; + if (!exported) { + exported = true; + try { + await super.export(metrics, (result: ExportResult) => { try { clearTimeout(timeout); // The test passes when the code is 0 because that means the @@ -470,6 +470,7 @@ describe('Bigtable/ClientSideMetrics', () => { assert.strictEqual(result.code, 0); resultCallback({code: 0}); void checkForPublishedMetrics(projectId) + // eslint-disable-next-line promise/always-return .then(() => { done(); }) @@ -480,12 +481,12 @@ describe('Bigtable/ClientSideMetrics', () => { // The code here isn't 0 so we report the original error to the mocha test runner. done(error); } - } else { - resultCallback({code: 0}); - } - }); - } catch (error) { - done(error); + }); + } catch (error) { + done(error); + } + } else { + resultCallback({code: 0}); } } } @@ -582,6 +583,7 @@ describe('Bigtable/ClientSideMetrics', () => { done: mocha.Done, onExportSuccess?: () => void, ) { + let exported = false; class TestExporter extends CloudMonitoringExporter { constructor(options: ClientOptions) { super(options); @@ -591,28 +593,32 @@ describe('Bigtable/ClientSideMetrics', () => { metrics: ResourceMetrics, resultCallback: (result: ExportResult) => void, ): Promise { - try { - await super.export(metrics, async (result: ExportResult) => { - try { - // The code is expected to be 0 because the - // result from calling export was successful. - assert.strictEqual(result.code, 0); - resultCallback({code: 0}); - if (onExportSuccess) { - onExportSuccess(); + if (!exported) { + exported = true; + try { + await super.export(metrics, async (result: ExportResult) => { + try { + // The code is expected to be 0 because the + // result from calling export was successful. + assert.strictEqual(result.code, 0); + resultCallback({code: 0}); + if (onExportSuccess) { + onExportSuccess(); + } + } catch { + // The code here isn't 0 so we report the original error to the + // mocha test runner. + // The test fails here because it means that an export was + // unsuccessful. + done(result); + resultCallback({code: 0}); } - } catch (error) { - // The code here isn't 0 so we report the original error to the - // mocha test runner. - // The test fails here because it means that an export was - // unsuccessful. - done(result); - done(error); - resultCallback({code: 0}); - } - }); - } catch (error) { - done(error); + }); + } catch (error) { + done(error); + resultCallback({code: 0}); + } + } else { resultCallback({code: 0}); } } diff --git a/handwritten/bigtable/test/table.ts b/handwritten/bigtable/test/table.ts index c10e89145d4..19f1abd4656 100644 --- a/handwritten/bigtable/test/table.ts +++ b/handwritten/bigtable/test/table.ts @@ -2072,12 +2072,12 @@ describe('Bigtable/Table', () => { let responses: any[] = []; beforeEach(() => { - clock = sinon.useFakeTimers({ + clock = sandbox.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'], }); - setTimeoutSpy = sinon.spy(global, 'setTimeout'); - clearTimeoutSpy = sinon.spy(global, 'clearTimeout'); - checkConsistencySpy = sinon.spy(table, 'checkConsistency'); + setTimeoutSpy = sandbox.spy(global, 'setTimeout'); + clearTimeoutSpy = sandbox.spy(global, 'clearTimeout'); + checkConsistencySpy = sandbox.spy(table, 'checkConsistency'); table.bigtable.request = (config: {}, callback: Function) => { responses.shift()(config, callback); @@ -2085,7 +2085,7 @@ describe('Bigtable/Table', () => { }); afterEach(() => { - clock.restore(); + sandbox.restore(); }); it('should return true if token is consistent', done => {