fix: remove TestMempoolFIFOWithParallelCheckTx (cometbft#2364)

This test fails 100% of the time. When new developers begin to use comet, the tests will not pass. Because they've been taught that tests not passing is a **bad thing** indicative of problems since most new users of CometBFT are / have been software engineers for some time, this makes the product less attractive. --- #### PR checklist - [x] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments - [x] Title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec --------- Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
fabiotudone-da · Feb 20, 2024 · a54fc7f · a54fc7f
1 parent 2c6e0c1
commit a54fc7f
Showing 1 changed file with 5 additions and 4 deletions.
diff --git a/mempool/reactor_test.go b/mempool/reactor_test.go
@@ -3,7 +3,6 @@ package mempool
 import (
 	"encoding/hex"
 	"errors"
-	"os"
 	"sync"
 	"testing"
 	"time"
@@ -334,10 +333,12 @@ func TestReactorTxSendersMultiNode(t *testing.T) {
 	require.Zero(t, len(firstReactor.txSenders))
 }
 
+// Finding a solution for guaranteeing FIFO ordering is not easy; it would
+// require changes at the p2p level. The order of messages is just best-effort,
+// but this is not documented anywhere. If this is well understood and
+// documented, we don't need this test. Until then, let's keep the test.
 func TestMempoolFIFOWithParallelCheckTx(t *testing.T) {
-	if os.Getenv("CI") != "" {
-		t.Skip("FIFO is not supposed to be guaranteed and this this is just used to evidence one of the cases where it does not happen. Hence we skip this test during CI.")
-	}
+	t.Skip("FIFO is not supposed to be guaranteed and this this is just used to evidence one of the cases where it does not happen. Hence we skip this test.")
 
 	config := cfg.TestConfig()
 	reactors, _ := makeAndConnectReactors(config, 4)