CA-142595: Fix, unit tests and refactoring #216
Closed
Conversation
This file contains 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
| return os.path.basename(devs[0]) | ||
| else: | ||
| return INVALID_DEVICE_NAME | ||
|
|
||
| def _extract_dev(device_dir, procname, host, target): | ||
| """Returns device name and creates dictionary entry for it""" | ||
| dev = _extract_dev_name(device_dir) |
There was a problem hiding this comment.
Could we rename _get_block_device_name to something like _get_block_device_name_kernel_3x to communicate that it's the kernel 3.x specific way?
|
@fquesnel - could you please review my changes and suggest any changes? also, could you please squash your commits to one, looking at the diff, it looks sensible as one change. Other than that, it looks good to me. |
Add a testing framework to enable us to check what happens if a given (SCSI) device does not have a block subdirectory. Signed-off-by: Mate Lakat <[email protected]>
Some device (e.g. a scsi security manager device.) has no subdir "block/" , this will cause _extract_dev_name "IndexError: list index out of range" error. The following is the traceback info: run this cmd: xe sr-probe type=lvmohba There was an SR backend failure. status: non-zero exit stdout: stderr: Traceback (most recent call last): File "/opt/xensource/sm/LVMoHBASR", line 239, in ? SRCommand.run(LVHDoHBASR, DRIVER_INFO) File "/opt/xensource/sm/SRCommand.py", line 343, in run sr = driver(cmd, cmd.sr_uuid) File "/opt/xensource/sm/SR.py", line 139, in init self.load(sr_uuid) File "/opt/xensource/sm/LVMoHBASR", line 98, in load print >>sys.stderr,self.hbasr.print_devs() File "/opt/xensource/sm/HBASR.py", line 242, in print_devs self._init_hbadict() File "/opt/xensource/sm/HBASR.py", line 63, in _init_hbadict dict = devscan.adapters(filterstr=self.type) File "/opt/xensource/sm/devscan.py", line 75, in adapters (dev, entry) = _extract_dev(dir, proc, id, lun) File "/opt/xensource/sm/devscan.py", line 235, in _extract_dev dev = _extract_dev_name(device_dir) File "/opt/xensource/sm/devscan.py", line 226, in _extract_dev_name dev = glob.glob(os.path.join(device_dir, 'block/*'))[0] IndexError: list index out of range Signed-off-by: Wang Yanbin [email protected]
fquesnel
force-pushed
the
test-xs64bit-CA-142595
branch
2 times, most recently
from
5efb3aa to
8701f04
Compare
|
@matelakat - I reviewed your changes and it looks good to me. Thanks, |
This was referenced Aug 28, 2014
|
Could you please also add the [UnitTest] string to each of the changes that deal with Unit Tests? Like this: d10e3b0 |
added 3 commits
September 3, 2014 12:24
Add unit tests relating to the fix proposed by Wang Yanbin. Signed-off-by: Flavien Quesnel <[email protected]>
Refactor code from devscan.py Signed-off-by: Flavien Quesnel <[email protected]>
Add unit tests, following the refactoring of devscan.py Signed-off-by: Flavien Quesnel <[email protected]>
fquesnel
force-pushed
the
test-xs64bit-CA-142595
branch
from
8701f04 to
84ad9da
Compare
|
OK, done. |
|
Merged via 41aeac9 |
|
Thank you. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This pull request includes:
-The fix from @wangyanbn (#203)
-The testing framework from @matelakat (#208)
-The unit tests for the fix, which are built on top of the testing framework
-Some additional refactoring, to prevent side effects when applying the fix
Thanks,
Flavien