Skip to content

Conversation

@danieljbruce
Copy link
Contributor

@danieljbruce danieljbruce commented Nov 18, 2024

Summary:

This PR adds a required Github action that ensures all conformance tests pass except for the conformance tests listed in the known_failures.txt file. This way we can be sure that changes won't be made in the future that cause conformance tests to fail that were passing before without us knowing.

Background:

This "conformance test PR" added a Github action that checks to see if all conformance tests are passing. This Github action fails even if one conformance test fails, but many of the conformance tests are failing at this time. So this existing Github action is useful for tracking the overall status of the conformance tests, but we need another Github action to ensure that addressing a known failure doesn't cause other conformance tests to fail.

Changes:

.github/sync-repo-settings.yaml: The new Github action called mandatory-conformance is added as a required status check in the continuous integration pipeline. This check makes it so we cannot merge PRs when a code change causes a passing conformance test to suddenly begin failing without adding that test to the known_failures.txt list. This will ensure that we do not merge PRs that introduce failing conformance tests without being explicit about introducing the test failure.

.github/workflows/mandatory-conformance.yaml: This file was created by copying conformance.yaml from this "conformance test PR" and then changing every mention of conformance to mandatory-conformance to introduce a new Github action that must pass in order for the PR to be merged.

.kokoro/mandatory-conformance.sh: This file was created by copying conformance.sh from this "conformance test PR" and then changing the line of code that runs the conformance tests to only run tests that are not in the known_failures.txt list. The line of code that prints the logs is also omitted so that it doesn't overwrite the logs produced in conformance.sh.

known_failures.txt: In this file a backslash is added to address an error and each test is now listed on a separate line so that each time a PR addresses a specific conformance test the diff will show us exactly what test has been addressed. If the test names are not on separate lines then it may be challenging to see exactly what tests have been added or removed to the known failures list. Also, this Github action revealed a list of other conformance tests that are failing that have been added to the list including:
TestReadRows_Retry_StreamReset
TestCheckAndMutateRow_Generic_Headers
TestMutateRows_Generic_DeadlineExceeded
TestReadRows_Generic_CloseClient
TestReadRows_Generic_DeadlineExceeded
TestReadRows_Retry_WithRoutingCookie
TestReadRows_Retry_WithRoutingCookie_MultipleErrorResponses
TestReadRows_Retry_WithRetryInfo
TestReadRows_Retry_WithRetryInfo_MultipleErrorResponse
TestSampleRowKeys_Generic_Headers
TestSampleRowKeys_Generic_DeadlineExceeded
TestSampleRowKeys_Retry_WithRoutingCookie

Next Steps:

In future PRs we will address one conformance test at a time shortening the known_failures.txt list with each conformance test we successful change from failing to passing.

@generated-files-bot
Copy link

Warning: This pull request is touching the following templated files:

  • .kokoro/mandatory-conformance.sh - .kokoro files are templated and should be updated in synthtool

@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: bigtable Issues related to the googleapis/nodejs-bigtable API. labels Nov 18, 2024
@danieljbruce danieljbruce changed the title Add the files needed for the mandatory conformance test: Add the files needed for the mandatory conformance Nov 18, 2024
@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/

@danieljbruce danieljbruce added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 18, 2024
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 18, 2024
- "ci/kokoro: Samples test"
- "ci/kokoro: System test"
- lint
- mandatory-conformance
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the new required Github action

@@ -1 +1,29 @@
TestMutateRow_Generic_Headers\|TestMutateRow_Generic_DeadlineExceeded|TestMutateRows_Generic_CloseClient\|TestMutateRows_Retry_WithRoutingCookie\|TestReadModifyWriteRow_Generic_Headers\|TestReadModifyWriteRow_NoRetry_TransientError\|TestReadModifyWriteRow_Generic_DeadlineExceeded\|TestReadRow_Generic_DeadlineExceeded\|TestReadRow_Retry_WithRoutingCookie\|TestReadRow_Retry_WithRetryInfo\|TestReadRows_ReverseScans_FeatureFlag_Enabled\|TestReadRows_NoRetry_OutOfOrderError_Reverse\|TestReadRows_Retry_PausedScan\|TestReadRows_Retry_LastScannedRow\|TestReadRows_Retry_LastScannedRow_Reverse\|TestCheckAndMutateRow_NoRetry_TransientError\|TestCheckAndMutateRow_Generic_DeadlineExceeded
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changes:

  1. Backslash after TestMutateRow_Generic_DeadlineExceeded
  2. All test names on their own line
  3. Lots of tests added to the file that are actually failing

@danieljbruce danieljbruce marked this pull request as ready for review November 18, 2024 20:21
@danieljbruce danieljbruce requested review from a team as code owners November 18, 2024 20:21
@danieljbruce danieljbruce changed the title test: Add the files needed for the mandatory conformance test: Add a new Github action to track failing conformance tests Nov 18, 2024
@danieljbruce danieljbruce added the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 19, 2024
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 19, 2024
@kevkim-codes kevkim-codes merged commit 6569c41 into googleapis:main Nov 22, 2024
17 of 21 checks passed
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: m Pull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants