test: add coverage for `-bantime` #26604

The assert_equal function should be called with the expected value as the first argument and the actual value as the second argument.

        assert_equal(1234, banned['ban_duration'])

It is generally a good idea to include a descriptive error message in the call to assert_equal so that it is clear what the error is if the assertion fails.

        assert_equal(1234, banned['ban_duration'], "ban_duration is not equal to 1234")

The assertion will already print this when hit

The assertion will already print this when hit

Yes, there is no need to include a message.

Pretty sure this will lead to a race when the rpc is dispatched after a one second sleep?

Where is/why the one second sleep?

Ah sorry. You are checking the ban_duration, not time_remaining. So this is fine. For reference, the output with the CPU put to sleep for a second looked like:

{
"address": "127.0.0.1/32", 
"ban_created": 1682845373, 
"banned_until": 1682846607, 
"ban_duration": 1234, 
"time_remaining": 1233
}

The code should check the return value of self.nodes[1].listbanned() to make sure it is not empty before trying to access the first element. If the list is empty, then the ban on 127.0.0.1 was not successful, and the code should fail the test.

It would be more readable if the code assigned the return value of self.nodes[1].listbanned() to a variable, and then accessed the first element of the list using that variable.

        banned_list = self.nodes[1].listbanned()
        assert len(banned_list) > 0, "Failed to ban 127.0.0.1"
        banned = banned_list[0]

[i] already asserts and fails the test

tAck but how about adding another coverage to test the ban remaining time after the bantime elapsed.

time.sleep(secs) banned = self.nodes[1].listbanned()[0] assert_equal(banned["time_remaining"],0)