-
Notifications
You must be signed in to change notification settings - Fork 6
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
Adding mocks and unit tests for socket code #63
Adding mocks and unit tests for socket code #63
Conversation
Pycode style issues need to be resolved dss_datamover/socket_communication.py:404:1: W391 blank line at end of file |
dss_datamover/tests/conftest.py
Outdated
return True | ||
|
||
def recv(self): | ||
# TODO: finish building out MockSocket class |
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.
remove this comment
dss_datamover/tests/conftest.py
Outdated
def __init__(self): | ||
self.host = "xxx.xxxx.xxxx.xxxx" | ||
self.port = "xxxx" | ||
STATUS_NORMAL = 0 |
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.
these DEFAULTS should be near the top of the file
if not self.data: | ||
return ret | ||
if self.data_index >= len(self.data): | ||
raise Exception |
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.
can we raise a specific exception here?
dss_datamover/tests/conftest.py
Outdated
return list(self.data.items()) | ||
else: | ||
ret = [] | ||
for k in self.data.keys(): |
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.
the specific key should only show up once. You can check to see if it is a nested dict, if so, just the items() method will pack the dict into a list of tuples
return self.data[k].items() if isinstance(self.data[k], dict) else [(k, self.data[k])]
|
||
@pytest.mark.usefixtures("get_mock_clientsocket") | ||
@pytest.mark.usefixtures("get_pytest_configs", "get_config_dict", "get_mock_logger", "get_header_length") | ||
class TestSocketCommunication(): |
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.
Great job on the tests, only recommendation I have is to consolidate them more, meaning that if there are tests that test the positive & negative & corner cases of the same functionality, such as socket connect. Then all of those variations should be within that same test case
@@ -55,7 +55,7 @@ def __init__(self, config, logger=None): | |||
self.logger = logger | |||
self.config = config | |||
self.socket = None | |||
self.last_exception_log_time = datetime.now() | |||
self.last_exception_log_time = datetime.min |
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.
change is not necessary please remove
@@ -232,7 +232,7 @@ def __init__(self, config, logger=None): | |||
self.logger = logger | |||
self.config = config | |||
self.client_socket = None | |||
self.last_exception_log_time = datetime.now() | |||
self.last_exception_log_time = datetime.min |
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.
change is not necessary please remove
…ile to pass sonar cloud security check.
Pycode style issue needs to be resolved ; dss_datamover/socket_communication.py:404:1: W391 blank line at end of file |
Sonar Cloud security hotspot dss_datamover/tests/conftest.py line 209. Make sure ip address is safe |
…security check & removing changes to socket_communication.py
Kudos, SonarCloud Quality Gate passed! 0 Bugs 0.0% Coverage The version of Java (11.0.17) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. |
Eric, please recreate PR under your own user and pull in master branch |
closing, as Eric has included changes in another PR |
No description provided.