You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
With host delete (#236) functionality, unittest.mock was added to the API tests. Although mocking is a good thing, we are not using unittest.mock in the API tests. If we were, we could mock out many things, test function calls, possibly even replace the database. But we don’t.
I suggest using one of there two approaches:
Use the mock class without unittest.mock. Make it so that it replaces KafkaProducer in tests.
Actually use the real Kafka message queue, just use different topics.
I used the former approach in my improvement suggestion branch. It’s more like a driver then, replacing Kafka with a dummy, and it’s similar to how Flask offers its test_client.
The latter one is more similar to how we use a real test database to store and read the records. It’s a dependency, but so is the database, the could be mocked out, but isn’t.
I think we should clearly define the scope and purpose of this test suite and that should determine the way we solve this issue.
With host delete (#236) functionality, unittest.mock was added to the API tests. Although mocking is a good thing, we are not using unittest.mock in the API tests. If we were, we could mock out many things, test function calls, possibly even replace the database. But we don’t.
I suggest using one of there two approaches:
I used the former approach in my improvement suggestion branch. It’s more like a driver then, replacing Kafka with a dummy, and it’s similar to how Flask offers its test_client.
The latter one is more similar to how we use a real test database to store and read the records. It’s a dependency, but so is the database, the could be mocked out, but isn’t.
I think we should clearly define the scope and purpose of this test suite and that should determine the way we solve this issue.
(Copy of my inline comment at #238.)
The text was updated successfully, but these errors were encountered: