Unity tests of logger and devices #142
Conversation
- Implements tests for handlers classes - Refactory some classes updating parameters
- Updates the comments of functions - Refactory of function parameters - Add new unit tests
Codecov Report
@@ Coverage Diff @@
## development #142 +/- ##
==============================================
Coverage ? 73.54%
==============================================
Files ? 17
Lines ? 1705
Branches ? 0
==============================================
Hits ? 1254
Misses ? 451
Partials ? 0
Continue to review full report at Codecov.
|
DeviceManager/ImportHandler.py
Outdated
| def __init__(self): | ||
| pass | ||
|
|
||
| @classmethod | ||
| def verifyInstance(cls, kafka): |
There was a problem hiding this comment.
The verifyInstance method repeats in several places. Maybe it could be in one place where the handles extend, like a class. I do not know well.
And maybe it's something more like a getInstance
There was a problem hiding this comment.
Make sense. I will improve this instantiation. Thanks!
There was a problem hiding this comment.
I don't know much about python.
But I thought more a parent class (for example Handler.py) where all the other somethingHandler could extend them.
And in it would remain in an attribute an instance, without the need to pass the cls, something like a singleton. Maybe it could just use something like a singleton, without the issue of inheritance either. But as I said, I don't know much about python and your solution seems ok to me. @kevinuehara
- Improves instanciation of kafka based on suggested comment - Updates the unit tests
|
Could you remove the '/test' folder from the statistics? |
|
|
||
| KafkaHandler().configure(device, meta={"service": 'admin'}) | ||
|
|
||
| def test_verify_intance_kafka(self): |
There was a problem hiding this comment.
Could you add some check to see if it really happened as expected in the above methods?
There was a problem hiding this comment.
I added a check that validates if the function passed the kafka flush in the next commit. Thanks!
There was a problem hiding this comment.
I also removed the "/test" folder from the coverage tests :)
| 'created': '2019-08-29T18:18:07.801602+00:00'} | ||
|
|
||
| with patch.object(KafkaInstanceHandler, "getInstance", return_value=MagicMock()): | ||
| ImportHandler().notifies_deletion_to_kafka('test_device', 'admin') |
There was a problem hiding this comment.
Could you see if the expected occurred?
There was a problem hiding this comment.
I added a check that validates if the function passed the kafka flush remove event in the next commit. Thanks!
| 'created': '2019-08-29T18:18:07.801602+00:00'} | ||
|
|
||
| with patch.object(KafkaInstanceHandler, "getInstance", return_value=MagicMock()): | ||
| ImportHandler().notifies_creation_to_kafka( |
There was a problem hiding this comment.
Could you see if the expected occurred?
There was a problem hiding this comment.
I added a check that validates if the function passed the kafka flush create event in the next commit. Thanks!
Description of PR:
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
What is the current behavior? (You can also link to an open issue here)
What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
Is there any issue related to this PR in other repository? (such as dojot/dojot)
Other information: