-
Notifications
You must be signed in to change notification settings - Fork 15
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
ci: improve robustness of functional tests #184
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
eeaton
changed the title
CI: correct issue with regexp in functional tests
ci: correct issue with regexp in functional tests
Jan 9, 2025
…n Cloud Build trigger substitution for CI automation. I clarified the documentation and made manual changes to the trigger outside of this PR.
… central cicd project. Reduce the steps so I can run just the functional test with subsitution from Cloud Build
…GoogleCloudPlatform/enterprise-knowledge-solution into ci-functional-test-improvements
…e out and cause very slow tests. Subsequent retries are no more likely to succeed than the first try.
… examples that work seem to be 9m, so it the current 10m timeout is probably too aggressive
…which is painful to modify and troubleshoot. 1: move sleep and retry logic into functional_test.go. 2: separate steps to prepare_functional and do_functional 3: separate helper script for adding cross-project IAM role to classifier service account, referencing terraform outputs
…iggered after a flaky failure
…lang code to exit instead of catching the error
…ry a possible error in the command
…me cmd object will fail on subsequent attempts.
and TestDAGIsSuccess
eeaton
changed the title
ci: correct issue with regexp in functional tests
ci: improve robustness of functional tests
Jan 16, 2025
…rt.EventuallyWith is a built-in wrapper that helps handle this use case; the previous method of manually repeating assert in loops doesn't work well for polling because a single failed assert method will fail the whole test.
… even if the previous stage TestDagIsAvailable has passed. Might be another propagation issue
…INTJ.pdf HLXB.pdf that may trigger to timeout
… 35+ of more which was at the high end of my timers
eyalbenivri
approved these changes
Jan 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I identified an issue in the regexp for functional test checking for the result of airflow dag statuses. I had a syntax issue in regexp that meant it was greedily matching too many characters, and would pass a test even when the status was "fail". This investigation also led to a few other improvements: