-
Notifications
You must be signed in to change notification settings - Fork 306
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
DAOS-6895 tests: Fix BadEvict test in weekly run. #4805
Conversation
Fixing Badevict test in weekly run. Use dmg instead of DaosApi for pool evict. Test-tag-vm: pr,-hw badevict Signed-off-by: Saurabh Tandan <saurabh.tandan@intel.com>
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. No errors found by checkpatch.
Test stage Functional_Hardware_Large completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-4805/1/execution/node/1529/log |
Test stage Functional_Hardware_Small completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-4805/2/execution/node/507/log |
Test stage Functional_Hardware_Medium completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-4805/2/execution/node/550/log |
Test stage Functional_Hardware_Large completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-4805/2/execution/node/689/log |
Test stage Functional on CentOS 7 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-4805/2/execution/node/765/log |
Test-tag-vm: pr,-hw badevict
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. No errors found by checkpatch.
FYI: Errors found in lines not modified in the patch:
src/tests/ftest/pool/bad_evict.py:63:
(pylint-consider-using-enumerate) Consider using enumerate instead of iterating with range and len
src/tests/ftest/pool/bad_evict.py:69:
(pylint-consider-using-enumerate) Consider using enumerate instead of iterating with range and len
src/tests/ftest/util/test_utils_pool.py:39:
(pylint-super-with-arguments) Consider using Python 3 style super() without arguments
src/tests/ftest/util/test_utils_pool.py:126:
(pylint-unnecessary-comprehension) Unnecessary use of a comprehension
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
@@ -71,12 +71,12 @@ def test_evict(self): | |||
pool.pool.uuid[4] = 244 | |||
|
|||
# evict the pool | |||
pool.pool.evict() | |||
self.get_dmg_command().pool_evict(pool.pool.get_uuid_str()) |
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.
self.get_dmg_command().pool_evict(pool.pool.get_uuid_str()) | |
pool.evict() |
Call pool.evict() if you want to use your new evict(). I guess you can use get_dmg_command(), but the original test is trying to do everything through the TestPool instance, so I think using your new evict() makes more sense. If you use your new evict(), I believe you need to set control_method: dmg under pool in the yaml.
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.
I just added self.pool.evict() for future use as I noticed it was missing but I think there need to be a lot of re-structuring of the test in order to use this method. So pool.evict() calls self.pool.uuid and if I use that, it will continue to give a continuous pass (even for null, and invalid uuids) and won't use the updated pool.uuid as it is inheriting original self.pool object.
But if we want to use invalid, null and actually self.pool.uuid we need to call self.get_dmg_command().pool_evict().
Having said that, I agree it is doable but will just need a whole lot of re-jig.
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
Failing issues are unrelated to the changes made in this pr. |
Fixing Badevict test in weekly run. Use dmg instead of DaosApi for pool evict. Test-tag-vm: pr,-hw badevict Signed-off-by: Saurabh Tandan <saurabh.tandan@intel.com>
Fixing Badevict test in weekly run.
Use dmg instead of DaosApi for pool evict.
Test-tag-vm: pr,-hw badevict
Signed-off-by: Saurabh Tandan saurabh.tandan@intel.com