From 3be5a37cd372f413856d8022e0c24f2b13b3bb35 Mon Sep 17 00:00:00 2001
From: Boris Nagaev <bnagaev@gmail.com>
Date: Thu, 30 May 2024 15:13:46 -0300
Subject: [PATCH] liquidity: fix flaky autoloop test

This failure became normal recently:

=== RUN   TestAutoLoopInEnabled
    autoloop_testcontext_test.go:318:
        	Error Trace:	/home/runner/work/loop/loop/liquidity/autoloop_testcontext_test.go:318
        	            				/home/runner/work/loop/loop/liquidity/autoloop_test.go:804
        	Error:      	Not equal:
        	            	expected: 80000
        	            	actual  : 160000
        	Test:       	TestAutoLoopInEnabled
    autoloop_testcontext_test.go:318:
        	Error Trace:	/home/runner/work/loop/loop/liquidity/autoloop_testcontext_test.go:318
        	            				/home/runner/work/loop/loop/liquidity/autoloop_test.go:804
        	Error:      	Not equal:
        	            	expected: 160000
        	            	actual  : 80000
        	Test:       	TestAutoLoopInEnabled
    autoloop_testcontext_test.go:343:
        	Error Trace:	/home/runner/work/loop/loop/liquidity/autoloop_testcontext_test.go:343
        	            				/home/runner/work/loop/loop/liquidity/autoloop_test.go:804
        	Error:      	Should be true
        	Test:       	TestAutoLoopInEnabled

The root cause is them the order of items in c.quoteRequestIn depends on the
order of loopInBuilder.buildSwap calls, which depends on the order of channel
handling in Manager.SuggestSwaps, which depends on the order of map traversal,
which is not determitistic.

Since in the test all the amounts are different, I used amount as a key and
put the expected calls into a map using amount as a key. When I extract an
item from c.quoteRequestIn channel, I find the corresponding item in the map
and remove it. All other logic is preserved.
---
 liquidity/autoloop_testcontext_test.go | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/liquidity/autoloop_testcontext_test.go b/liquidity/autoloop_testcontext_test.go
index 35e49ee7..cd4d767c 100644
--- a/liquidity/autoloop_testcontext_test.go
+++ b/liquidity/autoloop_testcontext_test.go
@@ -312,9 +312,26 @@ func (c *autoloopTestCtx) autoloop(step *autoloopStep) {
 	// Assert that we query the server for a quote for each of our
 	// recommended swaps. Note that this differs from our set of expected
 	// swaps because we may get quotes for suggested swaps but then just
-	// log them.
+	// log them. The order in c.quoteRequestIn is not deterministic,
+	// it depends on the order of map traversal (map peerChannels in
+	// method Manager.SuggestSwaps). So receive from the channel an item
+	// and then find a corresponding expected item, using amount as a key.
+	amt2expected := make(map[btcutil.Amount]quoteInRequestResp)
 	for _, expected := range step.quotesIn {
+		// Make sure all amounts are unique.
+		require.NotContains(c.t, amt2expected, expected.request.Amount)
+
+		amt2expected[expected.request.Amount] = expected
+	}
+
+	for i := 0; i < len(step.quotesIn); i++ {
 		request := <-c.quoteRequestIn
+
+		// Get the expected item, using amount as a key.
+		expected, has := amt2expected[request.Amount]
+		require.True(c.t, has)
+		delete(amt2expected, request.Amount)
+
 		assert.Equal(
 			c.t, expected.request.Amount, request.Amount,
 		)