Skip to content
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

Add unmanaged_tablet_test.sh to run a new e2e build on CI #578

Merged
merged 3 commits into from
Jul 18, 2024

Conversation

yoheimuta
Copy link
Contributor

@yoheimuta yoheimuta commented Jul 16, 2024

Description

This PR adds a new e2e test to ensure unmanaged tablet runs on the latest operator.

You see the following output on my personal build space:

Related Issue(s)

fixes: #577

… the future Vitess operator

Signed-off-by: Yohei Yoshimuta <[email protected]>
@@ -107,7 +107,7 @@ from the pull request, so in order to have the tag on the release branche's hist

##### Update the test output

The `upgrade_test.sh`, `backup_restore_test.sh` and `vtorc_vtadmin_test.sh` files must be updated with the proper release increment. Change the `verifyVtGateVersion` function calls to use the proper version (current version being released and latest previous version (only used in `upgrade_test.sh`)).
The `upgrade_test.sh`, `backup_restore_test.sh`, `vtorc_vtadmin_test.sh` and `unmanaged_tablet_test.sh` files must be updated with the proper release increment. Change the `verifyVtGateVersion` function calls to use the proper version (current version being released and latest previous version (only used in `upgrade_test.sh`)).
Copy link
Member

Choose a reason for hiding this comment

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

note to self, we will also need to update the vitess-releaser with the new file

Comment on lines +53 to +57
assertSelect ../common/select_commerce_data.sql "commerce" << EOF
Using commerce
Customer
+-------------+--------------------+
| customer_id | email |
Copy link
Member

Choose a reason for hiding this comment

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

Not necessarily in this Pull Request, but ideally we should extract some of the reused code into sharded shell functions. For instance all the code between line 28 and line 82 is mostly the same as in the four other tests. The function get_started() (in utils.sh) already extract all this code, but makes assumptions that are not always valid for every test, like the fact that vtorc is always here and that we expect a fixed number of vttablets.

@frouioui
Copy link
Member

It seems the test is failing because the branch does not contain the bug fix. I will merge main into this branch, which should turn the test green.

Copy link
Member

@frouioui frouioui left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks a bunch for adding this test.

Copy link
Collaborator

@deepthi deepthi left a comment

Choose a reason for hiding this comment

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

I agree with @frouioui about refactoring duplicated code.

@frouioui frouioui force-pushed the add-unmanaged-tablet-e2e branch from 2bed3a5 to c1bf796 Compare July 18, 2024 20:07
@frouioui frouioui merged commit d654572 into planetscale:main Jul 18, 2024
11 checks passed
@frouioui
Copy link
Member

frouioui commented Jul 18, 2024

I have marked the new workflow as required in the branch protection rules.

@yoheimuta
Copy link
Contributor Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhancement: Add an e2e test to ensure unmanaged tablet runs on the future Vitess operator
3 participants