search

openstack-charmers / zaza-openstack-tests

OpenStack Charms Functional Test Library for Zaza
Apache License 2.0
7 stars 77 forks source link

time.sleep roundup on retries #46

Closed ryan-beisner closed 3 years ago

ryan-beisner commented 5 years ago

The test_all_clients_authenticated test in vault/tests.py uses a hard-coded sleep time, wrapped by a for loop to retry.

It would be better to use tenacity retries to be consistent with the established approach to similar needs in other tests. https://github.com/openstack-charmers/zaza-openstack-tests/blob/2244f131a058159a8329385b00d0157655328769/zaza/openstack/charm_tests/vault/tests.py#L131-L139

I feel the same about the is_initialized method in vault/utils.py: https://github.com/openstack-charmers/zaza-openstack-tests/blob/2244f131a058159a8329385b00d0157655328769/zaza/openstack/charm_tests/vault/utils.py#L129-L141

And, same for mysql/tests.py: https://github.com/openstack-charmers/zaza-openstack-tests/blob/2244f131a058159a8329385b00d0157655328769/zaza/openstack/charm_tests/mysql/tests.py#L380-L392

IMHO:

When we call time.sleep(), we need to affirm that we are introducing a possible race condition for our future selves. In the majority of cases where time.sleep() is sought, we should seek a different approach.

And if retries are what we're looking for, we should use a consistent and reusable approach, rather than constructing a one-off retry loop. That may require further distillation of the test procedure into one or more helper functions, but it will be better for it in the end.

ryan-beisner commented 5 years ago

Here is an example of a Tenacity retry:

https://github.com/openstack-charmers/zaza-openstack-tests/blob/2244f131a058159a8329385b00d0157655328769/zaza/openstack/charm_tests/octavia/tests.py#L81-L84

guoqiao commented 5 years ago

@ryan-beisner I am trying to fix this by #53 , review appreciated:)