Skip to content
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

tests(system): updates tests to identify & delete stale dataset resources #717

Merged
merged 3 commits into from
Jun 10, 2020

Conversation

steffnay
Copy link
Contributor

@steffnay steffnay commented Jun 9, 2020

  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

🦕

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 9, 2020
@codecov
Copy link

codecov bot commented Jun 9, 2020

Codecov Report

Merging #717 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #717   +/-   ##
=======================================
  Coverage   95.16%   95.16%           
=======================================
  Files           7        7           
  Lines        6307     6307           
  Branches      369      406   +37     
=======================================
  Hits         6002     6002           
  Misses        305      305           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 92599f9...854343a. Read the comment docs.

@stephenplusplus
Copy link
Contributor

I was able to only make this change from master and it seemed to be working:

diff --git a/system-test/bigquery.ts b/system-test/bigquery.ts
index fe7007e..b0e2d36 100644
--- a/system-test/bigquery.ts
+++ b/system-test/bigquery.ts
@@ -1699,9 +1699,9 @@ describe('BigQuery', () => {
     });
 
     const deleteDatasetPromises = datasets
-      .filter(dataset => {
-        const creationTime = dataset.metadata.creationTime;
-        return creationTime && isResourceStale(creationTime);
+      .filter(async dataset => {
+        const [metadata] = await dataset.getMetadata();
+        return isResourceStale(parseInt(metadata.creationTime));
       })
       .map(dataset => {
         return dataset.delete({force: true});

Good catch on the dataset.metadata object not holding the creationTime property.

I was looking at creationTime as documented here, and it says

string (int64 format)

Output only. The time when this dataset was created, in milliseconds since the epoch.

Is it true it could be undefined or something else?

@steffnay
Copy link
Contributor Author

@stephenplusplus Sweet, that's so much easier. creationTime should never be undefined. I think this fixes the issue.

@steffnay steffnay added kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jun 10, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 10, 2020
@steffnay steffnay merged commit 0a1a3e8 into googleapis:master Jun 10, 2020
@steffnay steffnay deleted the sys-tests branch June 10, 2020 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants