missing e2e tag in many itest files

[bap2pecs]

//go:build e2e
// +build e2e

is missing in many files inside itest/

for example, itest/consumer_e2e_test.go has the tag but itest/consumer_test_manager.go doesn't have it

this can confuse lint

#414 is related and I suggest to fix the two issues together

[SebastianElvis]

Only files that contain test cases need the e2e tag while stuff like itest/consumer_test_manager.go does not need it, no?

[bap2pecs]

@SebastianElvis not exactly. we also followed the current structure initially to just add the tag in itest/opstackl2/op_e2e_test.go but not in itest/opstackl2/op_test_manager.go

however, CI failed complaining about these two vars not used:

const (
	opFinalityGadgetContractPath = "../bytecode/op_finality_gadget_f149c8b.wasm"
	opConsumerId                 = "op-stack-l2-12345"
)

digging deeper, it's b/c in the Makefil PACKAGES_E2E=$(shell go list ./... | grep '/itest') will exclude the files with tags

so when linting, it won't see the test file and see those two vars not used.

I think this implies two things:

• we should tag both files in the e2etest_op package with the same tag
• currently CI linting fails to cover certain files

so I did two things to fix it:

• add the tag in op_test_manager.go
• fixed the Makefile to add and use PACKAGES_E2E_OP=$(shell go list -tags=e2e_op ./... | grep '/itest')

cc @maurolacy this also explained you other question in #418 (review)

[gusin13]

hmm i see, ideally shouldn't we also lint the files which have build tags? I see there are 2 problems

1. We want the test files to be excluded from the go build i.e why we use // go:build <some_tag> so the binary doesn't include these files
2. The golangci-lint excludes files with go build which causes a weird state, as mentioned in the description of this issue

Solution

1. Either we should add the go build tag to all the test related files (including test managers etc or any helpers) this is same as what @bap2pecs mentioned above iiuc
2. Or just add the build tag to _test files and also enable the linter to include files with build tag

wdyt?

[bap2pecs]

ideally shouldn't we also lint the files which have build tags

@gusin13 yes we should but there are some problems in existing files and that's why I didn't try to fix them in my PRs. if you try to add the build tags in the lint command, you will see some errors. It's not breaking anything because the CI currently ignores all of them.

[bap2pecs]

solution 2 probably won't work because for files (e.g. test manager) that don't have such tags will fail the normal linting process without tags because it won't know which other files use the methods and variables defined in them. i.e. the unused var situation I described above

I think solution 1 is the proper fix but it could be a non-trivial change i.e. currently there r some test-only function defined in those consumer files, if we suddenly add the tag to all test files, those functions might be considered unused so we will need to move them to test files

[bap2pecs]

@gusin13

to continue the conversation from #423 (comment)

test-e2e-op: clean-e2e install-babylond
	go test -mod=readonly -timeout=25m -v $(PACKAGES_E2E) -count=1 --tags=e2e_op

this seems to work b/c it ignores all op tests.

the reason to have PACKAGES_E2E_OP is we don't want to run all tests at development (it took over 10 minutes at each run)

the lint doesn't know about packages

that's not the case anymore because I added https://github.com/babylonchain/finality-provider/blob/base/consumer-chain-support/.golangci.yml

[gusin13]

@bap2pecs

this seems to work b/c it ignores all op tests.

interesting, you are right op tests are being ignored somehow wondering why doesn't this problem occur in

test-e2e-bcd: clean-e2e install-babylond install-bcd
	go test -mod=readonly -timeout=25m -v $(PACKAGES_E2E) -count=1 --tags=e2e_bcd

if i run above the bcd tests run fine.

the reason to have PACKAGES_E2E_OP is we don't want to run all tests at development (it took over 10 minutes at each run)

go test -mod=readonly -timeout=25m -v $(PACKAGES_E2E) -count=1 --tags=e2e_op
wouldn't this be taken care by --tags flag, it will only compile and run the tests which have the build tag so PACKAGES_E2E_OP is not helpful here?

[bap2pecs]

@gusin13

It doesn't occur in

test-e2e-bcd: clean-e2e install-babylond install-bcd
	go test -mod=readonly -timeout=25m -v $(PACKAGES_E2E) -count=1 --tags=e2e_bcd

b/c only itest/cosmwasm/bcd/bcd_consumer_e2e_test.go has the tag

if you try to add the tag in all the files in /bcd, then it will be ignored

so the right way is to create PACKAGES_E2E_BCD. it works now b/c the two problems: 1) PACKAGES_E2E was used; 2) missing some build tag in those files kind of resolve each other lol

[bap2pecs]

@gusin13

wouldn't this be taken care by --tags flag, it will only compile and run the tests which have the build tag so PACKAGES_E2E_OP is not helpful here?

it won't b/c PACKAGES_E2E=$(shell go list ./... | grep '/itest') will ignore the /opstackl2 folder

$go list ./... | grep '/itest'
github.com/babylonchain/finality-provider/itest
github.com/babylonchain/finality-provider/itest/babylon
github.com/babylonchain/finality-provider/itest/cosmwasm/bcd
github.com/babylonchain/finality-provider/itest/cosmwasm/wasmd

so adding --tags=e2e_op in go test is too late

this implies we need to use PACKAGES_E2E_OP to match it