Fix flaky libp2p tests

[guillaumemichel]

Addresses #103

[iand]

Hit the race that is fixed in #102

[guillaumemichel]

Test is failing because the remote peer (sched[1]) likely isn't receiving the request within 100ms (!!!), so it isn't adding anything to its queue, and hence has nothing to run at the next step and fails the test.

I am not able to reproduce this on my machine. I assume that if this thread sleeps longer, it gives more time to other threads, and the message can be received within the 100 ms.

[iand]

This is problematic and means we don't have deterministic tests. If we're tweaking test timings then we're not in a better position than using raw goroutines.

We should try and remove sleeps from all tests. As a side issue, why aren't these sleeps using the scheduler's clock?

[guillaumemichel]

@iand unfortunately we cannot get deterministic testing when using go-libp2p. When using go-libp2p to send messages over the network stack, multiple go routines are at play, and we cannot use the scheduler to obtain sequentiality.

We should try and remove sleeps from all tests

I wish we were able to do it, but we need to wait for the message to be passed from one go routine to another over the network stack.

This test is testing the request timeout. A is sending a request to B. No action is run in B's scheduler until A's request times out. After A has timed out, B will send a late reply, and A should discard it. However, sometimes B's scheduler has no action to run after A's timeout of 100 ms (!!!).

There are 3 go routines at play:

1. The single worker, running both A and B schedulers.
2. A's go-libp2p go routine sending the request.
3. B's go-libp2p go routine listening at incoming requests.

It means that either (2) is not sending the request within 100ms or that (3) isn't processing the request within 100ms (, or that the request got lost in the local network stack, but it seems unlikely). It is very hard to understand what is wrong, because I cannot reproduce the failure on my machine (that is certainly faster than GH CI), and it only happens once in a while on GH CI.

I will try to add verbose when the test fails, and run it many times in GH CI.

As a side issue, why aren't these sleeps using the scheduler's clock?

You are right! In this case however it won't solve the flakiness, because the scheduler's clock is a real clock.

[guillaumemichel]

I am confident that TestConnections is fixed. The issue may have been introduced with go-libp2p 0.28, because loopback addresses may not be advertised anymore.

I am less confident with TestReqTimeout, because I am unable to reproduce the failure on my machine. I gave it a shot, and for now, it isn't failing on the CI (even though it only failed rarely). I will keep running the CI many times, and if the test isn't failing I would suggest that we merge it. If the same test is failing again in the future, we can open a new PR.

[iand]

You could run the following to reproduce the flake:

go test -c . && while ./libp2p.test -test.shuffle=on -test.count=100 -test.failfast; do date; done

[guillaumemichel]

TestReqTimeout isn't fixed. See https://github.com/plprobelab/go-kademlia/actions/runs/5892795414/job/16000386471#step:14:1

[guillaumemichel]

You could run the following to reproduce the flake:

go test -c . && while ./libp2p.test -test.shuffle=on -test.count=100 -test.failfast; do date; done

Unfortunately it doesn't help, the test keeps passing on my machine.

I have even run the following script for quite some time without getting the test to fail.

while true; do
    output=$(go1.20 test -run ^TestReqTimeout$)

    # Count the number of lines in the output
    line_count=$(echo "$output" | wc -l)

    # If the line count is greater than 2, the test failed. break out of the loop
    if (( line_count > 2 )); then
        echo "$output"
        break
    fi
done

[guillaumemichel]

TestReqTimeout alone took more than 1s to run on Windows Run Tests (32-bit). The issue probably comes from the timing, will try to reproduce the failure.

ok  	github.com/plprobelab/go-kademlia/libp2p	1.136s

[guillaumemichel]

The tests have run successfully 15 times in addition to the displayed tests.

IMO this is good to merge

[iand]

LGTM