-
Notifications
You must be signed in to change notification settings - Fork 3k
Added interface version information to mbed detect command. #5077
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
Changes from 8 commits
d43a847
7ff57c4
e4b55dd
256ab61
aafe614
7b49591
b887bb7
4c2d17e
a047f62
bb79f69
61f46b8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
""" | ||
mbed SDK | ||
Copyright (c) 2017 ARM Limited | ||
|
||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
You may obtain a copy of the License at | ||
|
||
http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
Unless required by applicable law or agreed to in writing, software | ||
distributed under the License is distributed on an "AS IS" BASIS, | ||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
See the License for the specific language governing permissions and | ||
limitations under the License. | ||
""" | ||
|
||
import unittest | ||
from tools.detect_targets import get_interface_version | ||
from tools.test_api import get_autodetected_MUTS_list | ||
|
||
""" | ||
Tests for detect_targets.py | ||
""" | ||
|
||
class DetectTargetsTests(unittest.TestCase): | ||
""" | ||
Test cases for Detect Target functionality | ||
""" | ||
|
||
def setUp(self): | ||
""" | ||
Called before each test case | ||
|
||
:return: | ||
""" | ||
# Gather a valid mount point from the host machine | ||
muts = get_autodetected_MUTS_list() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be mocked so that we don't depend on this test machine having some connected devices. |
||
|
||
count = 0 | ||
|
||
for mut in muts.values(): | ||
|
||
count += 1 | ||
self.valid_mount_point = mut['disk'] | ||
break | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks very out of place. Count is only ever going to be 1 or 0. |
||
|
||
# If no targets are found, there is no way to determine Host OS mount point. | ||
# Function should handle failure gracefully regardless of a mount point being present. | ||
# Therefore it is safe to assume a valid mount point. | ||
if count is 0: | ||
self.valid_mount_point = "D:" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is needed by the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How could we handle the case where there was no mount point (no device connected to the host), and across cases where host OS differs.. IE a valid mount point on Win 10 could be D: but a valid mount point in Linux is something like /media/{username}/DAPLink? There would need to be some condition that has to make an assumption at a certain point. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Test 'em all?
Yes, but that assumption can be guaranteed with mocking. |
||
|
||
self.invalid_mount_point = "23z/e\n" | ||
self.missing_mount_point = None | ||
|
||
def tearDown(self): | ||
""" | ||
Nothing to tear down. | ||
Called after each test case | ||
|
||
:return: | ||
""" | ||
pass | ||
|
||
def test_interface_version_valid_mount_point(self): | ||
|
||
interface_version = get_interface_version(self.valid_mount_point) | ||
assert len(interface_version) > 0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not a strong guarantee. I would like to see an assertion like the others in this commit: asserting that the interface version is a particular string/number. |
||
|
||
def test_interface_version_invalid_mount_point(self): | ||
|
||
interface_version = get_interface_version(self.invalid_mount_point) | ||
assert interface_version == 'unknown' | ||
|
||
def test_interface_version_missing_mount_point(self): | ||
|
||
interface_version = get_interface_version(self.missing_mount_point) | ||
assert interface_version == 'unknown' | ||
|
||
|
||
if __name__ == '__main__': | ||
unittest.main() |
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'm still not clear to me why I need an example of what to do with the parameter here. Are you suggesting that a user might be able to acquire a
mount_point
with the example code? If so, could you state that?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.
Pushed a new commit with a bit more clarity, apologies for the confusion ;)