Skip to content

ceph-volume: Fix TypeError: join() takes exactly one argument (2 given)#25469

Merged
alfredodeza merged 1 commit intoceph:masterfrom
sebastian-philipp:ceph-volume-fix-join
Dec 11, 2018
Merged

ceph-volume: Fix TypeError: join() takes exactly one argument (2 given)#25469
alfredodeza merged 1 commit intoceph:masterfrom
sebastian-philipp:ceph-volume-fix-join

Conversation

@sebastian-philipp
Copy link
Contributor

@sebastian-philipp sebastian-philipp commented Dec 10, 2018

Signed-off-by: Sebastian Wagner sebastian.wagner@suse.com
Fixes: http://tracker.ceph.com/issues/37595

@LenzGr
Copy link
Contributor

LenzGr commented Dec 10, 2018

Fixing a regression introduced in #25425

@alfredodeza
Copy link
Contributor

This needs to have unit tests associated with this function. Otherwise regressions like the one this PR is fixing can keep going undetected. Please include the unit tests needed to cover all cases in this function.

@sebastian-philipp
Copy link
Contributor Author

This regression was found by #25236 which will add Teuthology testing to the orchestrator (which will invoke c-v). So, #25236 can already be seen as the integration test code for this PR.

Right now, I don't have time to add additional unit tests for this function. If you want to, I can look into adding unit tests for this PR tomorrow.

@alfredodeza
Copy link
Contributor

Sure, I don't mind other functional tests. But for sure we don't want to pursue more changes without having tests that verify the patch.

Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
@sebastian-philipp
Copy link
Contributor Author

Sure, I don't mind other functional tests. But for sure we don't want to pursue more changes without having tests that verify the patch.

Added a test for _get_device_id()

@alfredodeza
Copy link
Contributor

jenkins test ceph-volume tox

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants