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

Adding bootstrap mysqlrouter action test to mysqlrouter #1082

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

xtrusia
Copy link
Contributor

@xtrusia xtrusia commented Jul 19, 2023

Copy link
Contributor

@ajkavanagh ajkavanagh left a comment

Choose a reason for hiding this comment

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

Please add a verification test for the bootstrap action that checks that the action does (re-)bootstrap the mysql-router.

zaza/openstack/charm_tests/mysql/tests.py Outdated Show resolved Hide resolved
@xtrusia xtrusia force-pushed the master branch 4 times, most recently from 0c3b499 to 34205f6 Compare August 8, 2023 23:47
@xtrusia xtrusia requested a review from ajkavanagh August 9, 2023 00:03
@xtrusia
Copy link
Contributor Author

xtrusia commented Aug 29, 2023

Please review this case if you have time, thanks a lot!

Copy link
Contributor

@ajkavanagh ajkavanagh left a comment

Choose a reason for hiding this comment

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

This looks good; just an issue with potentially very long test times due to endless while loops.

zaza/openstack/charm_tests/mysql/tests.py Outdated Show resolved Hide resolved
zaza/openstack/charm_tests/mysql/tests.py Outdated Show resolved Hide resolved
ajkavanagh
ajkavanagh previously approved these changes Sep 1, 2023
@ajkavanagh ajkavanagh dismissed their stale review September 1, 2023 07:17

Prematurely clicked approve.

@xtrusia
Copy link
Contributor Author

xtrusia commented Oct 13, 2023

Could somebody review this one?

Thanks in advance

Copy link
Contributor

@ajkavanagh ajkavanagh left a comment

Choose a reason for hiding this comment

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

I'm still not clear on why the cls.conf_file has been changed as without a bit more explanation it's not obvious whether the test_910_restart_on_config_change actually does nothing (i.e. isn't a valid test) as the change to the cls.conf_file ought to affect the test in some way.

zaza/openstack/charm_tests/mysql/tests.py Outdated Show resolved Hide resolved
zaza/openstack/charm_tests/mysql/tests.py Outdated Show resolved Hide resolved
@xtrusia
Copy link
Contributor Author

xtrusia commented Nov 6, 2023

I'm still not clear on why the cls.conf_file has been changed as without a bit more explanation it's not obvious whether the test_910_restart_on_config_change actually does nothing (i.e. isn't a valid test) as the change to the cls.conf_file ought to affect the test in some way.

I changed it back to original and I put another variable for specific test. could you please review it? Thanks!

@xtrusia
Copy link
Contributor Author

xtrusia commented Nov 18, 2023

@ajkavanagh any chance to review this case? I get it back to original and added new variable for only new test.
Thanks in advance!

@xtrusia xtrusia marked this pull request as draft March 5, 2024 09:31
@xtrusia xtrusia marked this pull request as ready for review March 5, 2024 09:31
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.

None yet

2 participants