-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Replace ErrorContains checks with Error checks before running upgrade downgrade #16688
Replace ErrorContains checks with Error checks before running upgrade downgrade #16688
Conversation
…ks to Error checks Signed-off-by: Manan Gupta <[email protected]>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Signed-off-by: Manan Gupta <[email protected]>
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.
There could be a possible use of EqualError
as well.
Let's discourage that for end to end test.
Signed-off-by: Manan Gupta <[email protected]>
Good point, I updated the PR to also change |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16688 +/- ##
==========================================
- Coverage 68.95% 68.93% -0.02%
==========================================
Files 1565 1565
Lines 201530 201556 +26
==========================================
- Hits 138957 138943 -14
- Misses 62573 62613 +40 ☔ View full report in Codecov by Sentry. |
… downgrade (vitessio#16688) Signed-off-by: Manan Gupta <[email protected]>
… downgrade (vitessio#16688) Signed-off-by: Manan Gupta <[email protected]>
… downgrade (vitessio#16688) Signed-off-by: Manan Gupta <[email protected]>
Description
This PR replaces the
ErrorContains
checks to just check for anError
instead while running upgrade downgrade tests. This is required because we don't guarantee that errors will be the same across different vitess version.The workings of the changes can be seen in the workflow run https://github.com/vitessio/vitess/actions/runs/10630690740/job/29470087067?pr=16688 in the
Convert ErrorContains to Error checks
step. I am copying a snippet so that it can be seen directly here -This change is required so that we can get this CI check to work https://github.com/vitessio/vitess/actions/runs/10488116666/job/29052695413?pr=16619. This works in conjunction with #16672 that fixes some anti-patterns in the test.
Related Issue(s)
Checklist
Deployment Notes