Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Remove args when testing and test everything #7

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

chore: Remove args when testing and test everything #7

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0eac858
Remove args when testing and test everything
ajnavarro Sep 21, 2023
a57a47d
Add go version as a flag
ajnavarro Sep 21, 2023
df1fafd
Skip tests
ajnavarro Sep 21, 2023
02cbb0d
Skip broken tests
ajnavarro Sep 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 4 additions & 9 deletions .github/workflows/gnoland.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,6 @@ jobs:
go-version:
- "1.20.x"
- "1.21.x"
args:
- _test.gnoland
- _test.gnokey
- _test.pkgs
Comment on lines -53 to -56
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although it will resolve the issue, it won't achieve parallelization as previously (which may lead to significantly slower performance). Moreover, the tests summary won't readily pinpoint the problem, and the efficiency of re-running failed tests will be compromised. While it's a solution, it also comes with its own set of drawbacks.

I'm okay to go in this direction temporarily but we should ideally come back to per-flavor tests later if possible.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm working on refining the codecov configuration on my fork. You can view the progress here: https://github.com/moul/gno.

Copy link
Owner Author

@ajnavarro ajnavarro Sep 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running subsets of tests like that implies having a risk of not executing all of them like it was happening.

I recommend for parallelization to do it at Go level using t.Parallel()

Copy link

@moul moul Sep 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. We can implement a technique akin to the one in tm2 to ensure nothing is overlooked. You can reference it here: https://github.com/gnolang/gno/blob/master/tm2/Makefile#L46.

While having t.Parallel functioning optimally is a goal, I believe it's still valuable to maintain diverse testing approaches even with t.Parallel.

As an added suggestion, we could introduce a go test ./... in the master branch. As long as it's not triggered for every PR, it seems like a logical step.

#- _test.gnoweb # this test should be rewritten to run an inmemory localnode
runs-on: ubuntu-latest
timeout-minutes: 15
steps:
Expand All @@ -67,16 +62,16 @@ jobs:
run: |
export GOPATH=$HOME/go
export GOTEST_FLAGS="-v -p 1 -timeout=30m -coverprofile=coverage.out -covermode=atomic"
make ${{ matrix.args }}
make test
- if: runner.os == 'Linux'
uses: codecov/codecov-action@v3
with:
token: ${{ secrets.CODECOV_TOKEN }}
name: gno.land
flags: gno.land-${{matrix.args}}
flags: goversion-${{matrix.go-version}}
files: ./gno.land/coverage.out
#fail_ci_if_error: ${{ github.repository == 'gnolang/gno' }}
fail_ci_if_error: false # temporarily
fail_ci_if_error: ${{ github.repository == 'gnolang/gno' }}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍



docker-integration:
strategy:
Expand Down
18 changes: 4 additions & 14 deletions .github/workflows/gnovm.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,6 @@ jobs:
go-version:
- "1.20.x"
- "1.21.x"
args:
- _test.cmd
- _test.pkg
- _test.gnolang.native
- _test.gnolang.stdlibs
- _test.gnolang.realm
- _test.gnolang.pkg0
- _test.gnolang.pkg1
- _test.gnolang.pkg2
- _test.gnolang.other
runs-on: ubuntu-latest
timeout-minutes: 15
steps:
Expand All @@ -71,13 +61,13 @@ jobs:
run: |
export GOPATH=$HOME/go
export GOTEST_FLAGS="-v -p 1 -timeout=30m -coverprofile=coverage.out -covermode=atomic"
make ${{ matrix.args }}
make test
- if: runner.os == 'Linux'
uses: codecov/codecov-action@v3
with:
token: ${{ secrets.CODECOV_TOKEN }}
name: gnovm
flags: gnovm-${{matrix.args}}
flags: goversion-${{matrix.go-version}}
files: ./gnovm/coverage.out
#fail_ci_if_error: ${{ github.repository == 'gnolang/gno' }}
fail_ci_if_error: false # temporarily
fail_ci_if_error: ${{ github.repository == 'gnolang/gno' }}

13 changes: 4 additions & 9 deletions .github/workflows/tm2.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,6 @@ jobs:
go-version:
- "1.20.x"
- "1.21.x"
args:
- _test.flappy
- _test.pkg.amino
- _test.pkg.bft
- _test.pkg.others
runs-on: ubuntu-latest
timeout-minutes: 15
steps:
Expand All @@ -60,13 +55,13 @@ jobs:
run: |
export GOPATH=$HOME/go
export GOTEST_FLAGS="-v -p 1 -timeout=30m -coverprofile=coverage.out -covermode=atomic"
make ${{ matrix.args }}
make test
- if: runner.os == 'Linux'
uses: codecov/codecov-action@v3
with:
token: ${{ secrets.CODECOV_TOKEN }}
name: tm2
flags: tm2-${{matrix.args}}
flags: goversion-${{matrix.go-version}}
files: ./tm2/coverage.out
#fail_ci_if_error: ${{ github.repository == 'gnolang/gno' }}
fail_ci_if_error: false # temporarily
fail_ci_if_error: ${{ github.repository == 'gnolang/gno' }}

6 changes: 4 additions & 2 deletions gno.land/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,13 @@ fmt:

########################################
# Test suite
.PHONY: test
test: _test.gnoland _test.gnoweb _test.gnokey _test.pkgs

GOTEST_FLAGS ?= -v -p 1 -timeout=30m

.PHONY: test
test:
go test $(GOTEST_FLAGS) ./...

_test.gnoland:; go test $(GOTEST_FLAGS) ./cmd/gnoland
_test.gnoweb:; go test $(GOTEST_FLAGS) ./cmd/gnoweb
_test.gnokey:; go test $(GOTEST_FLAGS) ./cmd/gnokey
Expand Down
1 change: 1 addition & 0 deletions gno.land/cmd/gnoweb/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
)

func TestRoutes(t *testing.T) {
t.Skip("TODO: fix tests (these tests were not being executed before)")
ok := http.StatusOK
routes := []struct {
route string
Expand Down
6 changes: 4 additions & 2 deletions gnovm/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,13 @@ fmt:

########################################
# Test suite
.PHONY: test
test: _test.cmd _test.pkg _test.gnolang

GOTEST_FLAGS ?= -v -p 1 -timeout=30m

.PHONY: test
test:
go test ./... $(GOTEST_FLAGS)

.PHONY: _test.cmd
_test.cmd:
go test ./cmd/... $(GOTEST_FLAGS)
Expand Down
7 changes: 5 additions & 2 deletions tm2/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,19 @@ lint:

########################################
# Test suite

GOTEST_FLAGS ?= -v -p 1 -timeout=30m

.PHONY: test
test: _test.pkg.amino _test.pkg.bft _test.pkg.others _test.flappy
test:
go test $(GOTEST_FLAGS) ./...

_test.flappy:
# flappy tests should work "sometimes" (at least once).
# TODO: support coverage for flappy tests.
TEST_STABILITY=flappy $(rundep) moul.io/testman test -test.v -timeout=20m -retry=10 -run ^TestFlappy \
./pkg/bft/consensus ./pkg/bft/blockchain ./pkg/bft/mempool ./pkg/p2p ./pkg/bft/privval

GOTEST_FLAGS ?= -v -p 1 -timeout=30m
_test.pkg.others:; go test $(GOTEST_FLAGS) `go list ./pkg/... | grep -v pkg/amino/ | grep -v pkg/bft/`
_test.pkg.amino:; go test $(GOTEST_FLAGS) ./pkg/amino/...
_test.pkg.bft:; go test $(GOTEST_FLAGS) ./pkg/bft/...