Skip to content

Commit

Permalink
chore: make CI green (#3114)
Browse files Browse the repository at this point in the history
<!--
Thank you for submitting a pull request!

Please make sure you have reviewed our contributors guide before
submitting your
first PR.

Please ensure you've addressed or included references to any related
issues.

Tips:
- Use keywords like "closes" or "fixes" followed by an issue number to
automatically close related issues when the PR is merged (e.g., "closes
#123" or "fixes #123").
- Describe the changes made in the PR.
- Ensure the PR has one of the required tags (kind:fix, kind:misc,
kind:break!, kind:refactor, kind:feat, kind:deps, kind:docs, kind:ci,
kind:chore, kind:testing)

-->

Building off @vgonkivs's [work to skip
tests](#2802), this
attempts to get a baseline "green" ci for us. There are still a couple
of tests that VERY intermittently flake in CI, but this way they will
stand out WHEN they happen and we can track down vs entire thing being
red always.

This does quite a few things

- introduces build tags on swamp tests in each named file to allow us to
run parts of the swamp/integration tests independently (ie: `go test
./... -tags=blob`
- adds an `integration` tag to allow running all still
- splits the integration tests into it's own workflow file
(`integration-tests.yml`) which is now triggered from `go-ci.yml`
- splits each swamp/integration tests to run as its own job so we can
see which are failing/flakey more explicitly
- utilizes go's -short test flag to Skip a few swamp/integration tests
that are consistently failing. Current we are skipping
`TestFullReconstructFromFulls`, `TestFullReconstructFromLights`,
`TestSyncStartStopLightWithBridge` which is less than we were originally
skipping in #2802
- plugs in our verbose/debug stuff to integration tests in addition to
unit which we had before

Unit tests 

- splits some of the unit tests that were "race flakey" into
`*_no_race_test.go` and adds `!race` tag to some others to get to pass
consistently when running unit tests with -race flag
- macos-latest unit tests still fail on race test ONLY in GitHub actions
CI 🤷‍♂️

Next Steps

- create issues for each short run / skipped 
- create issues for any tests that fail race that are NOT the race issue
on upstream cosmos-sdk
- create issue for macos-latest in GitHub race fail
- create issue for macos-latest intermittent fail

I think we let this run for a while, then once we see it be consistently
green for a bit and isolate any more flakes that pop up, we can toggle
on more branch requirements, then in fixing the above issues, we can be
green. [Being green is not
easy](https://www.youtube.com/watch?v=51BQfPeSK8k).
  • Loading branch information
ramin authored Jan 23, 2024
1 parent dd18169 commit ba969f9
Show file tree
Hide file tree
Showing 21 changed files with 390 additions and 201 deletions.
29 changes: 8 additions & 21 deletions .github/workflows/go-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ concurrency:

jobs:
setup:
name: Setup
runs-on: ubuntu-latest
outputs:
debug: ${{ steps.debug.outputs.debug }}
Expand Down Expand Up @@ -104,11 +105,10 @@ jobs:
file: ./coverage.txt
name: coverage-${{ matrix.os }}

unit_race_test:
unit_test_race:
needs: [lint, go_mod_tidy_check]
name: Run Unit Tests with Race Detector
name: Unit Tests with Race Detector (ubuntu-latest)
runs-on: ubuntu-latest
continue-on-error: true

steps:
- uses: actions/checkout@v4
Expand All @@ -119,24 +119,11 @@ jobs:
go-version: ${{ inputs.go-version }}

- name: execute test run
run: make test-unit-race
run: make test-unit-race ENABLE_VERBOSE=${{ needs.setup.outputs.debug }}

integration_test:
name: Integration Tests
needs: [lint, go_mod_tidy_check]
name: Run Integration Tests
runs-on: ubuntu-latest
continue-on-error: true

steps:
- uses: actions/checkout@v4

- name: set up go
uses: actions/setup-go@v5
with:
go-version: ${{ inputs.go-version }}

- name: Integration Tests
run: make test-integration

- name: Integration Tests with Race Detector
run: make test-integration-race
uses: ./.github/workflows/integration-tests.yml
with:
go-version: ${{ inputs.go-version }}
115 changes: 115 additions & 0 deletions .github/workflows/integration-tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
name: Integration Tests

on:
workflow_call:
inputs:
go-version:
description: 'Go version'
required: true
type: string

jobs:
api_tests:
name: Integration Tests API
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v4

- name: set up go
uses: actions/setup-go@v5
with:
go-version: ${{ inputs.go-version }}

- name: run API tests
run: make test-integration TAGS=api

blob_tests:
name: Integration Tests Blob
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v4

- name: set up go
uses: actions/setup-go@v5
with:
go-version: ${{ inputs.go-version }}

- name: run blob tests
run: make test-integration TAGS=blob

fraud_tests:
name: Integration Tests Fraud
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v4

- name: set up go
uses: actions/setup-go@v5
with:
go-version: ${{ inputs.go-version }}

- name: run fraud tests
run: make test-integration TAGS=fraud

nd_tests:
name: Integration Tests ND
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v4

- name: set up go
uses: actions/setup-go@v5
with:
go-version: ${{ inputs.go-version }}

- name: run nd tests
run: make test-integration TAGS=nd

p2p_tests:
name: Integration Tests p2p
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v4

- name: set up go
uses: actions/setup-go@v5
with:
go-version: ${{ inputs.go-version }}

- name: run p2p tests
run: make test-integration TAGS=p2p

reconstruction_tests:
name: Integration Tests Reconstruction
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v4

- name: set up go
uses: actions/setup-go@v5
with:
go-version: ${{ inputs.go-version }}

- name: run reconstruction tests
run: make test-integration SHORT=true TAGS=reconstruction

sync_tests:
name: Integration Tests Sync
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v4

- name: set up go
uses: actions/setup-go@v5
with:
go-version: ${{ inputs.go-version }}

- name: run sync tests
run: make test-integration SHORT=true TAGS=sync
27 changes: 12 additions & 15 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ PROJECTNAME=$(shell basename "$(PWD)")
DIR_FULLPATH=$(shell pwd)
versioningPath := "github.com/celestiaorg/celestia-node/nodebuilder/node"
LDFLAGS=-ldflags="-X '$(versioningPath).buildTime=$(shell date)' -X '$(versioningPath).lastCommit=$(shell git rev-parse HEAD)' -X '$(versioningPath).semanticVersion=$(shell git describe --tags --dirty=-dev 2>/dev/null || git rev-parse --abbrev-ref HEAD)'"
TAGS=integration
SHORT=
ifeq (${PREFIX},)
PREFIX := /usr/local
endif
Expand All @@ -13,6 +15,11 @@ else
VERBOSE =
LOG_AND_FILTER =
endif
ifeq ($(SHORT),true)
INTEGRATION_RUN_LENGTH = -short
else
INTEGRATION_RUN_LENGTH =
endif
## help: Get more info on make commands.
help: Makefile
@echo " Choose a command run in "$(PROJECTNAME)":"
Expand Down Expand Up @@ -118,29 +125,21 @@ test-unit:
## test-unit-race: Running unit tests with data race detector
test-unit-race:
@echo "--> Running unit tests with data race detector"
@go test -race `go list ./... | grep -v nodebuilder/tests`
@go test $(VERBOSE) -race -covermode=atomic -coverprofile=coverage.txt `go list ./... | grep -v nodebuilder/tests` $(LOG_AND_FILTER)
.PHONY: test-unit-race

## test-integration: Running /integration tests located in nodebuilder/tests
test-integration:
@echo "--> Running integrations tests"
@go test ./nodebuilder/tests
@echo "--> Running integrations tests $(VERBOSE) -tags=$(TAGS) $(INTEGRATION_RUN_LENGTH)"
@go test $(VERBOSE) -tags=$(TAGS) $(INTEGRATION_RUN_LENGTH) ./nodebuilder/tests
.PHONY: test-integration

## test-integration-race: Running integration tests with data race detector located in node/tests
test-integration-race:
@echo "--> Running integration tests with data race detector"
@go test -race ./nodebuilder/tests
@echo "--> Running integration tests with data race detector -tags=$(TAGS)"
@go test -race -tags=$(TAGS) ./nodebuilder/tests
.PHONY: test-integration-race

## test: Running both unit and integrations tests
test:
@echo "--> Running all tests without data race detector"
@go test ./...
@echo "--> Running all tests with data race detector"
@go test -race ./...
.PHONY: test

## benchmark: Running all benchmarks
benchmark:
@echo "--> Running benchmarks"
Expand All @@ -164,7 +163,6 @@ pb-gen:
done;
.PHONY: pb-gen


## openrpc-gen: Generate OpenRPC spec for Celestia-Node's RPC api
openrpc-gen:
@echo "--> Generating OpenRPC spec"
Expand Down Expand Up @@ -221,7 +219,6 @@ goreleaser-release:
.PHONY: goreleaser-release

# Copied from https://github.com/dgraph-io/badger/blob/main/Makefile

USER_ID = $(shell id -u)
HAS_JEMALLOC = $(shell test -f /usr/local/lib/libjemalloc.a && echo "jemalloc")
JEMALLOC_URL = "https://github.com/jemalloc/jemalloc/releases/download/5.2.1/jemalloc-5.2.1.tar.bz2"
Expand Down
55 changes: 55 additions & 0 deletions core/fetcher_no_race_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
//go:build !race

package core

import (
"context"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/tendermint/tendermint/types"
)

// TestBlockFetcherHeaderValues tests that both the Commit and ValidatorSet
// endpoints are working as intended.
func TestBlockFetcherHeaderValues(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), time.Second*10)
t.Cleanup(cancel)

client := StartTestNode(t).Client
fetcher := NewBlockFetcher(client)

// generate some blocks
newBlockChan, err := fetcher.SubscribeNewBlockEvent(ctx)
require.NoError(t, err)
// read once from channel to generate next block
var h int64
select {
case evt := <-newBlockChan:
h = evt.Header.Height
case <-ctx.Done():
require.NoError(t, ctx.Err())
}
// get Commit from current height
commit, err := fetcher.Commit(ctx, &h)
require.NoError(t, err)
// get ValidatorSet from current height
valSet, err := fetcher.ValidatorSet(ctx, &h)
require.NoError(t, err)
// get next block
var nextBlock types.EventDataSignedBlock
select {
case nextBlock = <-newBlockChan:
case <-ctx.Done():
require.NoError(t, ctx.Err())
}
// compare LastCommit from next block to Commit from first block height
assert.Equal(t, nextBlock.Header.LastCommitHash, commit.Hash())
assert.Equal(t, nextBlock.Header.Height, commit.Height+1)
// compare ValidatorSet hash to the ValidatorsHash from first block height
hexBytes := valSet.Hash()
assert.Equal(t, nextBlock.ValidatorSet.Hash(), hexBytes)
require.NoError(t, fetcher.UnsubscribeNewBlockEvent(ctx))
}
43 changes: 0 additions & 43 deletions core/fetcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/tendermint/tendermint/types"
)

func TestBlockFetcher_GetBlock_and_SubscribeNewBlockEvent(t *testing.T) {
Expand Down Expand Up @@ -38,45 +37,3 @@ func TestBlockFetcher_GetBlock_and_SubscribeNewBlockEvent(t *testing.T) {
}
require.NoError(t, fetcher.UnsubscribeNewBlockEvent(ctx))
}

// TestBlockFetcherHeaderValues tests that both the Commit and ValidatorSet
// endpoints are working as intended.
func TestBlockFetcherHeaderValues(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), time.Second*10)
t.Cleanup(cancel)

client := StartTestNode(t).Client
fetcher := NewBlockFetcher(client)

// generate some blocks
newBlockChan, err := fetcher.SubscribeNewBlockEvent(ctx)
require.NoError(t, err)
// read once from channel to generate next block
var h int64
select {
case evt := <-newBlockChan:
h = evt.Header.Height
case <-ctx.Done():
require.NoError(t, ctx.Err())
}
// get Commit from current height
commit, err := fetcher.Commit(ctx, &h)
require.NoError(t, err)
// get ValidatorSet from current height
valSet, err := fetcher.ValidatorSet(ctx, &h)
require.NoError(t, err)
// get next block
var nextBlock types.EventDataSignedBlock
select {
case nextBlock = <-newBlockChan:
case <-ctx.Done():
require.NoError(t, ctx.Err())
}
// compare LastCommit from next block to Commit from first block height
assert.Equal(t, nextBlock.Header.LastCommitHash, commit.Hash())
assert.Equal(t, nextBlock.Header.Height, commit.Height+1)
// compare ValidatorSet hash to the ValidatorsHash from first block height
hexBytes := valSet.Hash()
assert.Equal(t, nextBlock.ValidatorSet.Hash(), hexBytes)
require.NoError(t, fetcher.UnsubscribeNewBlockEvent(ctx))
}
Loading

0 comments on commit ba969f9

Please sign in to comment.