Wait, you are testing the output of cronjob.log here, but that is a MagicMock right? So this will always succeed! Or am I on the wrong track here?

I used MagicMock to make a mock object of cronjob.log so we dont actually log something, apart from that the behavior of it depends only on what is being logged by the upper methods that call it, so if your concern was did i mock and pass the log manually, so no that is actually not happening from my understanding. I feel this class can be improved a lot and the tests can be more clear with what they are doing. Thanks for the feedback!

Ah, ok. I think I understand. I think I was confused because I expected that it is tested which honeypot logged what. You verify that some info and warning were logged, but not that Log4pot got the info and Cowrie got the warning. Because this is what happens, right?

@regulartim i think i understand, as we know the name of the honeypot we can actually log the name of it in the assertion, the latest commit actually asserts equal with the number directly, but logging the name is also doable, might make the tests brittle if we do it but also enhances the code from readability point of view because we are clear just by reading whats happening. let me know what you prefer i would assert it accordingly.

I don't understand what this does. Could you please explain?

Yes! So if you refer line https://github.com/intelowlproject/GreedyBear/blob/develop/greedybear/cronjobs/monitor_logs.py#L27 we are checking the logs for 4 different pots, so stat_result which contains the time is called four times, so this function returns recent time for the first call and old time for rest. Hence later we assert that the send_message method was only called once with greedy bear as it was the only pot with latest log time.......this can be decoupled into two test functions surely but as already iterated by you that we prefer to follow the test format present, i took reference from test https://github.com/intelowlproject/GreedyBear/blob/develop/tests/greedybear/cronjobs/test_firehol.py where multiple test cases were written in side effect.

I would prefer multiple functions and each method is responsible for testing a specific behavior.

I am sorry, but I still find that function very confusing, although I do understand now why it is here and what it is doing. Would something like this be equivalent?

mock_stat.side_effect = [
       MagicMock(st_mtime=recent_time),
       MagicMock(st_mtime=old_time),
       MagicMock(st_mtime=old_time),
       MagicMock(st_mtime=old_time),
   ]

I think this is more explicit and easier to read and understand. What do you think?

Could you also write a test, where the log was not populated and nothing was sent?

yes! @regulartim would you prefer the general test design where we write multiple functions and each method is responsible for testing a specific behavior or would you prefer it to how it is currently and all the tests coupled in one single function using side effects?

I would prefer multiple functions and each method is responsible for testing a specific behavior.

I am sorry, but I still find that function very confusing, although I do understand now why it is here and what it is doing. Would something like this be equivalent?

mock_stat.side_effect = [
       MagicMock(st_mtime=recent_time),
       MagicMock(st_mtime=old_time),
       MagicMock(st_mtime=old_time),
       MagicMock(st_mtime=old_time),
   ]

I think this is more explicit and easier to read and understand. What do you think?