Skip to content

test: Add a logger to print the instance id.#1129

Merged
danieljbruce merged 4 commits intogoogleapis:mainfrom
danieljbruce:1128-logging-for-cluster-system-tests
Jul 18, 2022
Merged

test: Add a logger to print the instance id.#1129
danieljbruce merged 4 commits intogoogleapis:mainfrom
danieljbruce:1128-logging-for-cluster-system-tests

Conversation

@danieljbruce
Copy link
Contributor

This PR is used to help with debugging cluster issues.

Fixes #1128

@danieljbruce danieljbruce requested a review from a team as a code owner July 13, 2022 17:03
@danieljbruce danieljbruce requested a review from a team July 13, 2022 17:03
@product-auto-label product-auto-label bot added size: xs Pull request size is extra small. api: bigtable Issues related to the googleapis/nodejs-bigtable API. labels Jul 13, 2022
@danieljbruce danieljbruce changed the title Add a logger to print the instance id. feat: Add a logger to print the instance id. Jul 13, 2022
@conventional-commit-lint-gcf
Copy link

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

Copy link
Contributor

@kolea2 kolea2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please rename PR to "test: ..." prefix

},
});
console.log(`Test Instance Id: ${instanceId}`);
await operation.promise();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we log the cluster we query in checkMetadata (e.g. method starting line 40)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. We can log the cluster id.

@@ -67,6 +67,7 @@ describe('Cluster', () => {
time_created: Date.now(),
},
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it also possible to add more IDs on the cluster that is being compared in the assertion? i.e. to easily show which cluster is mismatching the ID

  1) Cluster << would be nice to have the cluster ID here >>
       Update cluster
         Starting from autoscaling
           should change cluster to manual scaling:

      AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

3 !== 5

      + expected - actual

      -3
      +5

      at checkMetadata (build/system-test/cluster.js:31:16)
          -> /workspace/system-test/cluster.ts:42:12
      at async Context.<anonymous> (build/system-test/cluster.js:273:17)
          -> /workspace/system-test/cluster.ts:306:9

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. I'm not sure how to make a dynamic title in the mocha framework, but we can definitely print out the cluster id.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries, yea, whatever is easier to do, as I'm not too familiar w/ Typescript and its frameworks. So long as we see the cluster ID logged earlier in the log that corresponds w/ the test failure, we should be alright

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good.

@danieljbruce danieljbruce added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 13, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 13, 2022
@danieljbruce danieljbruce changed the title feat: Add a logger to print the instance id. test: Add a logger to print the instance id. Jul 13, 2022
@danieljbruce danieljbruce added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 13, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 13, 2022
@danieljbruce danieljbruce added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 13, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 13, 2022
@danieljbruce danieljbruce added kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Jul 13, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Jul 13, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 13, 2022
@danieljbruce danieljbruce added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 14, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 18, 2022
@danieljbruce danieljbruce added the owlbot:run Add this label to trigger the Owlbot post processor. label Jul 18, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Jul 18, 2022
@danieljbruce danieljbruce merged commit 2005842 into googleapis:main Jul 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigtable Issues related to the googleapis/nodejs-bigtable API. size: xs Pull request size is extra small.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a log in the asserts for system tests on clusters

4 participants