Skip to content
This repository was archived by the owner on Mar 26, 2026. It is now read-only.

chore(testproxy): Address the TestReadModifyWriteRow_Generic_DeadlineExceeded conformance test#1543

Merged
gcf-merge-on-green[bot] merged 5 commits intomainfrom
380268110-deadline-exceeded-readrows-fix
Dec 13, 2024
Merged

chore(testproxy): Address the TestReadModifyWriteRow_Generic_DeadlineExceeded conformance test#1543
gcf-merge-on-green[bot] merged 5 commits intomainfrom
380268110-deadline-exceeded-readrows-fix

Conversation

@danieljbruce
Copy link
Copy Markdown
Contributor

@danieljbruce danieljbruce commented Dec 12, 2024

Summary

The conformance test passes a timeout setting into the createClient test proxy service. With this change the timeout setting will now be applied to the client and used in the readModifyWriteRow call. This allows the TestReadModifyWriteRow_Generic_DeadlineExceeded conformance test to pass because now the timeout is being used for these calls.

@danieljbruce danieljbruce requested review from a team December 12, 2024 15:48
@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 Dec 12, 2024
@danieljbruce danieljbruce marked this pull request as draft December 12, 2024 16:55
@product-auto-label product-auto-label Bot added size: s Pull request size is small. and removed size: xs Pull request size is extra small. labels Dec 12, 2024
const clientConfig = require('../../src/v2/bigtable_client_config.json');
const clientConfig = JSON.parse(
JSON.stringify(require('../../src/v2/bigtable_client_config.json'))
);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

To prevent conformance tests from modifying behaviour in other conformance tests, we must use a copy of the clientConfig so that we don't modify the original and set timeouts for tests that shouldn't be using timeouts.

TODO: In the future we should apply this for all methods, but right
now doing so results in regressions in the TestSampleRowKeys_Generic_MultiStreams
and TestSampleRowKeys_Generic_CloseClient conformance tests that need
to be addressed.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The regressions have been addressed by making a copy of the clientConfig so now this timeout can apply to all methods.

@danieljbruce danieljbruce added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 13, 2024
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 13, 2024
@kevkim-codes kevkim-codes marked this pull request as ready for review December 13, 2024 15:21
@danieljbruce danieljbruce added automerge Merge the pull request once unit tests and other checks pass. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Dec 13, 2024
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 13, 2024
@danieljbruce danieljbruce added the owlbot:run Add this label to trigger the Owlbot post processor. label Dec 13, 2024
@gcf-owl-bot gcf-owl-bot Bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Dec 13, 2024
@gcf-merge-on-green gcf-merge-on-green Bot merged commit 23a2b97 into main Dec 13, 2024
@gcf-merge-on-green gcf-merge-on-green Bot removed the automerge Merge the pull request once unit tests and other checks pass. label Dec 13, 2024
@gcf-merge-on-green gcf-merge-on-green Bot deleted the 380268110-deadline-exceeded-readrows-fix branch December 13, 2024 17:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants