Added read_write_int method and unit test for them #275
Conversation
injaleea
requested review from
Fladolcetta,
bhaswath,
marcoskirsch,
srinidhin and
texasaggie97-zz
src/nifake/tests/test_session.py
Outdated
|
|
||
| def test_get_vi_int32_attribute(self): | ||
| self.patched_library.niFake_GetAttributeViInt32.side_effect = self.side_effects_helper.niFake_GetAttributeViInt32 | ||
| int = 3 |
There was a problem hiding this comment.
I don't like naming variables the same as the type. Can you call this something different?
src/nifake/tests/test_session.py
Outdated
| assert(attr_int == int) | ||
| from mock import call | ||
| calls = [call(SESSION_NUM_FOR_TEST, b"", 1000004, ANY)] | ||
| self.patched_library.niFake_GetAttributeViInt32.assert_has_calls(calls) |
There was a problem hiding this comment.
Why does this test use mock.call to check the call, while the next test uses assert_called_once_with()?
src/nifake/tests/test_session.py
Outdated
| session1 = nifake.Session('dev1') | ||
| session.close() | ||
| session1.close() | ||
| assert True |
src/nifake/tests/test_session.py
Outdated
| self.patched_library.niFake_GetError.side_effect = self.side_effects_helper.niFake_GetError | ||
| self.side_effects_helper['GetError']['errorCode'] = test_error_code | ||
| self.side_effects_helper['GetError']['description'] = test_error_desc | ||
| session = nifake.Session('dev1') |
There was a problem hiding this comment.
Why do you need two sessions in this test?
src/nifake/tests/test_session.py
Outdated
| session.read_write_integer = int | ||
| self.patched_library.niFake_SetAttributeViInt32.assert_called_once_with(SESSION_NUM_FOR_TEST, b'', attribute_id, 1) | ||
|
|
||
| def test_error_on_close(self): |
There was a problem hiding this comment.
One of the things close() should do, is log on failure (cannot / we decided not to raise).
This should have been fixed #198 but we missed it.
| test_number = 1 | ||
| with nifake.Session('dev1') as session: | ||
| session.read_write_integer = test_number | ||
| self.patched_library.niFake_SetAttributeViInt32.assert_called_once_with(SESSION_NUM_FOR_TEST, b'', attribute_id, 1) |
There was a problem hiding this comment.
How come you didn't use test_number here?
src/nifake/tests/test_session.py
Outdated
| test_error_desc = 'Test' | ||
| self.patched_library.niFake_close.side_effect = self.side_effects_helper.niFake_close | ||
| self.side_effects_helper['close']['return'] = test_error_code | ||
| self.side_effects_helper['close']['vi'] = SESSION_NUM_FOR_TEST |
There was a problem hiding this comment.
What is this supposed to accomplish? niFake_Close() takes the ViSession as an input. You should be checking that with assert_called_once_with...
src/nifake/tests/test_session.py
Outdated
| self.side_effects_helper['GetError']['errorCode'] = test_error_code | ||
| self.side_effects_helper['GetError']['description'] = test_error_desc | ||
| session = nifake.Session('dev1') | ||
| session.close() |
There was a problem hiding this comment.
When close() errors, we don't raise an exception. But we do or at least should log a warning (don't remember if that's implemented yet). Can you check that in the test as well?
There was a problem hiding this comment.
#254 is what I was referring to.
| assert self.patched_errors.handle_error.call_count == 2 | ||
| self.patched_errors.handle_error.assert_called_with(session, self.patched_library.niFake_GetCalDateAndTime.return_value) | ||
| ''' | ||
|
|
There was a problem hiding this comment.
When there are changes to a test file, either system tests or unit tests, there should be a corresponding change in the generated files.
When you don't have the changed generated file as part of your change, it ends up being an unrelated change in a different pull request.
3 participants
[X] This contribution adheres to CONTRIBUTING.md.
What does this Pull Request accomplish?
Added fake method for read_write_integer attribute
Added unit test for set get integer attribute
unit test for close with error
Why should this Pull Request be merged?
More unit test coverage for fake
What testing has been done?
Passed on dev machine