Cover the RbdManager helper - #100
Merged
Merged
Conversation
insatomcat
force-pushed
the
rbd-unit-coverage
branch
from
September 7, 2026 19:16
3ebf503 to
437c01d
Compare
eroussy
approved these changes
Sep 14, 2026
109 unit tests against fake Ceph bindings, no cluster. helpers/rbd_manager.py goes from 17% to 98%. The stubs in tests/ceph_stubs.py keep raising on use for every other test file. This one replaces Rados, RBD, Image and Group inside rbd_manager alone, because testing the helper means running the code that builds them. Signed-off-by: Florent Carli <florent.carli@rte-france.com>
img_inst was bound inside the try, so an image that cannot be opened raised UnboundLocalError from the finally and masked the real error. Bind it before, as the other methods do. Signed-off-by: Florent Carli <florent.carli@rte-france.com>
A class constructor always returns an instance, so the "is None" branch could never run. rbd.Image and rbd.Group signal an absent object by raising, which the callers already propagate. Signed-off-by: Florent Carli <florent.carli@rte-france.com>
The handler caught RbdException, which this module raises and the Ceph bindings never do, so an init failure was never logged. Catch what the bindings raise instead. The error still propagates unchanged. Signed-off-by: Florent Carli <florent.carli@rte-france.com>
insatomcat
force-pushed
the
rbd-unit-coverage
branch
from
September 14, 2026 11:50
437c01d to
fd72e99
Compare
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second of three PRs on the helpers.
helpers/rbd_manager.pygoes from 17% to 100% statement and 100% branch coverage, in 114 unit tests, with no Ceph.Every other test file replaces
RbdManageras a whole, so the stubs intests/ceph_stubs.pycan keep raising on use: a test that reaches real Ceph code fails loudly rather than passing against a fake. This file is the exception, since testing the helper means running the code that buildsRados,RBD,ImageandGroup. Its fixture replaces those four names insiderbd_manageralone, for the duration of a test. Every other file keeps seeing the stubs that raise. The fakes share one state object thatImageandGroupreach through the I/O context they are handed, the way the real bindings do.Three defects turned up while writing the tests, each fixed in its own commit.
rollback_imageboundimg_instinside itstry, so an image that cannot be opened raisedUnboundLocalErrorfrom thefinallyand hid the real error. Same shape as theclonerollback fixed in #97._get_imageand_get_groupeach tested their freshly built instance againstNone. A class constructor always returns an instance, so those branches could never run;rbd.Imageandrbd.Groupsignal an absent object by raising, which the callers already propagate.The constructor caught
RbdExceptionaround the connection sequence. That is this module's own class, which the Ceph bindings never raise, so an init failure was never logged. It now catches what the bindings actually raise. This is the one behaviour change in the PR: a warning line now appears when a connection fails. The error itself still propagates unchanged.Nineteen mutations of the module are all caught, among them the unit exponent in the size conversion, the
overwriteanddeepbranches, theforcebranch of the snapshot purge, the group membership tests, and the pool name in theqemu-imgdestination.Branch coverage of the whole project reaches 82% on this branch. Independent of #99, which touches a different module.