Skip to content

Commit

Permalink
ci: fix NO_RACE + CI re-run on timeouts or fails of known flakes (#1304)
Browse files Browse the repository at this point in the history
* fix script

* fix

* suggested for ci: re-run tests if failures are flakes (#1305)

* suggested ci improvement: re-run tests if failures are flakes

* remove flake indicators

* bash

* add more flakes

* fix bash

* fix

* add flake

* sort
  • Loading branch information
darioush authored Aug 26, 2024
1 parent 32c8cdf commit 3ede00b
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 14 deletions.
3 changes: 0 additions & 3 deletions accounts/keystore/keystore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,9 +293,6 @@ type walletEvent struct {
// Tests that wallet notifications and correctly fired when accounts are added
// or deleted from the keystore.
func TestWalletNotifications(t *testing.T) {
if os.Getenv("RUN_FLAKY_TESTS") != "true" {
t.Skip("FLAKY")
}
t.Parallel()
_, ks := tmpKeyStore(t, false)

Expand Down
4 changes: 0 additions & 4 deletions metrics/sample_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package metrics
import (
"math"
"math/rand"
"os"
"runtime"
"testing"
"time"
Expand Down Expand Up @@ -133,9 +132,6 @@ func TestExpDecaySample(t *testing.T) {
// The priority becomes +Inf quickly after starting if this is done,
// effectively freezing the set of samples until a rescale step happens.
func TestExpDecaySampleNanosecondRegression(t *testing.T) {
if os.Getenv("RUN_FLAKY_TESTS") != "true" {
t.Skip("FLAKY")
}
sw := NewExpDecaySample(100, 0.99)
for i := 0; i < 100; i++ {
sw.Update(10)
Expand Down
4 changes: 0 additions & 4 deletions plugin/evm/gossiper_eth_gossiping_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"crypto/ecdsa"
"encoding/json"
"math/big"
"os"
"strings"
"sync"
"testing"
Expand Down Expand Up @@ -74,9 +73,6 @@ func getValidEthTxs(key *ecdsa.PrivateKey, count int, gasPrice *big.Int) []*type
// show that a geth tx discovered from gossip is requested to the same node that
// gossiped it
func TestMempoolEthTxsAppGossipHandling(t *testing.T) {
if os.Getenv("RUN_FLAKY_TESTS") != "true" {
t.Skip("FLAKY")
}
assert := assert.New(t)

key, err := crypto.GenerateKey()
Expand Down
1 change: 0 additions & 1 deletion plugin/evm/syncervm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,6 @@ func TestStateSyncToggleEnabledToDisabled(t *testing.T) {
}

func TestVMShutdownWhileSyncing(t *testing.T) {
t.Skip("FLAKY")
var (
lock sync.Mutex
vmSetup *syncVMSetup
Expand Down
38 changes: 36 additions & 2 deletions scripts/build_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,39 @@ race="-race"
if [[ -n "${NO_RACE:-}" ]]; then
race=""
fi
# shellcheck disable=SC2046
go test -shuffle=on "${race}" -timeout="${TIMEOUT:-600s}" -coverprofile=coverage.out -covermode=atomic "$@" $(go list ./... | grep -v github.com/ava-labs/subnet-evm/tests)

# MAX_RUNS bounds the attempts to retry the tests before giving up
# This is useful for flaky tests
MAX_RUNS=4
for ((i = 1; i <= MAX_RUNS; i++));
do
# shellcheck disable=SC2046
go test -shuffle=on ${race:-} -timeout="${TIMEOUT:-600s}" -coverprofile=coverage.out -covermode=atomic "$@" $(go list ./... | grep -v github.com/ava-labs/subnet-evm/tests) | tee test.out || command_status=$?

# If the test passed, exit
if [[ ${command_status:-0} == 0 ]]; then
rm test.out
exit 0
else
unset command_status # Clear the error code for the next run
fi

# If the test failed, print the output
unexpected_failures=$(
# First grep pattern corresponds to test failures, second pattern corresponds to test panics due to timeouts
(grep "^--- FAIL" test.out | awk '{print $3}' || grep -E '^\s+Test.+ \(' test.out | awk '{print $1}') |
sort -u | comm -23 - ./scripts/known_flakes.txt
)
if [ -n "${unexpected_failures}" ]; then
echo "Unexpected test failures: ${unexpected_failures}"
exit 1
fi

# Note the absence of unexpected failures cannot be indicative that we only need to run the tests that failed,
# for example a test may panic and cause subsequent tests in that package to not run.
# So we loop here.
echo "Test run $i failed with known flakes, retrying..."
done

# If we reach here, we have failed all retries
exit 1
8 changes: 8 additions & 0 deletions scripts/known_flakes.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
TestClientCancelWebsocket
TestExpDecaySampleNanosecondRegression
TestGolangBindings
TestMempoolEthTxsAppGossipHandling
TestTransactionSkipIndexing
TestVMShutdownWhileSyncing
TestWalletNotifications
TestWebsocketLargeRead

0 comments on commit 3ede00b

Please sign in to comment.