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.

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

Add E2ETestSuite Type (E2E #3) #1650

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
47be6be
feat(e2e): adding script to dynamically generate list of e2e tests
chatton Jul 4, 2022
ce80411
feat(e2e): adding github CI to run E2E tests
chatton Jul 4, 2022
bfbba03
feat(e2e) adding placeholder test for github action to execute
chatton Jul 4, 2022
4569fa5
wip: extracting correct tag for image
chatton Jul 4, 2022
272ad61
wip: extracting correct tag for image
chatton Jul 4, 2022
6afe90c
wip: corrected workflow syntax
chatton Jul 4, 2022
7497574
wip: corrected workflow syntax
chatton Jul 4, 2022
7091fbc
removed unreferenced job
chatton Jul 4, 2022
6ebcc6d
wip: adding makefile
chatton Jul 4, 2022
05d9a45
wip: displaying env vars in placeholder test
chatton Jul 4, 2022
308eb0f
wip: adding go script to determine simd tag
chatton Jul 4, 2022
9ebacd7
fix: corrected outputs name in test.yml
chatton Jul 4, 2022
c431850
fix: specifying correct output name
chatton Jul 4, 2022
3c4f62b
fix: updating go script to accept a single argument
chatton Jul 4, 2022
b5fff2a
chore: greatly simplifying determine_simd_tag.go
chatton Jul 4, 2022
abb0eca
chore: adding docstring to determine_simd_tag.go
chatton Jul 4, 2022
f36025b
chore: extract output variable
chatton Jul 4, 2022
9039137
chore: adding echo to display the tag in the workflow
chatton Jul 4, 2022
30591c9
feat(e2e): adding E2ETestSuite
chatton Jul 4, 2022
b43d07e
wip: temporarily remove entrypoint in dockerfile
chatton Jul 4, 2022
b1bcc12
chore: removing SkipPathCreation
chatton Jul 4, 2022
d740296
removed Req and eRep member fields
chatton Jul 4, 2022
e94d35b
merge main
chatton Jul 6, 2022
848f888
chore: adding docstrings and cleaning up E2ETestSuite
chatton Jul 6, 2022
99328b1
merge main
chatton Jul 8, 2022
35e454f
chore: correcting docstring
chatton Jul 8, 2022
ccd9fd5
chore: updating test.yml to pass only the pr
chatton Jul 8, 2022
e154f09
chore: upgrading go to 1.18 for e2e test
chatton Jul 8, 2022
236eb9f
Merge branch 'main' into cian/issue#1648-add-baseline-e2e-test-suite-…
chatton Jul 12, 2022
11227eb
Merge branch 'main' into cian/issue#1648-add-baseline-e2e-test-suite-…
chatton Jul 13, 2022
7541beb
Merge branch 'main' into cian/issue#1648-add-baseline-e2e-test-suite-…
chatton Jul 13, 2022
e768a46
chore: addressing PR feedback
chatton Jul 14, 2022
ce2828e
Merge branch 'cian/issue#1648-add-baseline-e2e-test-suite-type-(e2e-#…
chatton Jul 14, 2022
dcd5048
Merge branch 'main' into cian/issue#1648-add-baseline-e2e-test-suite-…
chatton Jul 14, 2022
1fcd668
Merge branch 'main' into cian/issue#1648-add-baseline-e2e-test-suite-…
chatton Jul 14, 2022
67b1a8b
chore: adressing PR feedback
chatton Jul 14, 2022
07f7015
Merge branch 'cian/issue#1648-add-baseline-e2e-test-suite-type-(e2e-#…
chatton Jul 14, 2022
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
26 changes: 9 additions & 17 deletions .github/scripts/determine_simd_tag.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,32 +3,24 @@ package main
import (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had misunderstood the behaviour of the ref/pr values in GithubActions. I simplified this script to simply accept a PR number as a string, or use "main" if we are not in a PR.

"flag"
"fmt"
"os"
)

var prNum int
var ref string
var prNum string

func init() {
flag.IntVar(&prNum, "pr", 0, "the number of the pr")
flag.StringVar(&ref, "ref", "", "the github ref")
flag.StringVar(&prNum, "pr", "", "the number of the pr")
flag.Parse()
}

// in the context of a GithubAction workflow, the PR is the event number. So if the ref is not specified
// but the event number is, that means we are running for a PR. If the ref is specified, this means
// we have merged the PR, so we want to use the ref as a tag instead of the PR number.
// in the context of a GithubAction workflow, the PR is non empty if it is a pr. When
// code is merged to main, it will be empty. In this case we just use the "main" tag.
func main() {
if prNum == 0 && ref == "" {
fmt.Printf("must specify one or bot of [pr, ref]")
os.Exit(1)
}
fmt.Printf(getSimdTag(prNum, ref))
fmt.Printf(getSimdTag(prNum))
}

func getSimdTag(prNum int, ref string) string {
if ref != "" {
return ref
func getSimdTag(prNum string) string {
if prNum == "" {
return "main"
}
return fmt.Sprintf("pr-%d", prNum)
return fmt.Sprintf("pr-%s", prNum)
}
5 changes: 4 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ jobs:
go-version: 1.18
- id: get-tag
run: |
tag=$(go run .github/scripts/determine_simd_tag.go -ref "${{ github.event.push.ref }}" -pr "${{ github.event.pull_request.number }}" )
tag=$(go run .github/scripts/determine_simd_tag.go -pr "${{ github.event.pull_request.number }}" )
echo "Using tag $tag"
echo "::set-output name=simd-tag::$tag"

Expand All @@ -233,6 +233,9 @@ jobs:
matrix: ${{ fromJSON(needs.build-test-matrix.outputs.matrix) }}
steps:
- uses: actions/checkout@v3
- uses: actions/setup-go@v3
with:
go-version: 1.18
- name: Log in to the Container registry
uses: docker/login-action@49ed152c8eca782a232dede0303416e8f356c37b
with:
Expand Down
3 changes: 2 additions & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,5 @@ FROM ubuntu:20.04

COPY --from=builder /go/build/simd /bin/simd

ENTRYPOINT ["simd"]
# TODO(chatton): uncomment once https://github.com/strangelove-ventures/ibctest/issues/183 is resolved.
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
#ENTRYPOINT ["simd"]
28 changes: 18 additions & 10 deletions e2e/fee_middleware_test.go
Original file line number Diff line number Diff line change
@@ -1,29 +1,37 @@
package e2e

import (
"os"
"context"
"testing"

"github.com/strangelove-ventures/ibctest/ibc"
"github.com/stretchr/testify/suite"

"github.com/cosmos/ibc-go/v4/e2e/testsuite"
)

func TestFeeMiddlewareTestSuite(t *testing.T) {
suite.Run(t, new(FeeMiddlewareTestSuite))
}

type FeeMiddlewareTestSuite struct {
suite.Suite
testsuite.E2ETestSuite
}

func (s *FeeMiddlewareTestSuite) TestPlaceholder() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the place holder test now creates chains in a container and starts a relayer (no assertions are made yet!)

tag, ok := os.LookupEnv("SIMD_TAG")
s.Require().True(ok)
s.T().Logf("SIMD_TAG=%s", tag)
ctx := context.Background()
r := s.SetupChainsRelayerAndChannel(ctx, feeMiddlewareChannelOptions())
s.T().Run("start relayer", func(t *testing.T) {
s.StartRelayer(r)
})

image, ok := os.LookupEnv("SIMD_IMAGE")
s.Require().True(ok)
s.T().Logf("SIMD_IMAGE=%s", image)
}

s.T().Logf("Placeholder test")
s.Require().True(true)
// feeMiddlewareChannelOptions configures both of the chains to have fee middleware enabled.
func feeMiddlewareChannelOptions() func(options *ibc.CreateChannelOptions) {
return func(opts *ibc.CreateChannelOptions) {
opts.Version = "{\"fee_version\":\"ics29-1\",\"app_version\":\"ics20-1\"}"
opts.DestPortName = "transfer"
opts.SourcePortName = "transfer"
Comment on lines +34 to +35
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: rename to avoid dest/source naming. Maybe in a followup issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one is trickier as it is part of the ibctest library.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there already an issue created? Would you like me to open one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't created an issue against the ibctest repo yet for this. The src/dst naming convention is widespread throughout the repo so it would be a relatively significant change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea you are right. Do you think it is too much work to request changes for?

In the past I've spent a fair amount of time debugging the difference between source/destination, that it seems worthwhile to me to consider changing naming conventions if a library is going to have increased usage. Happy to let the maintainers decide if they want to do the changes, but the "source" and "destination" terminology doesn't make much sense when packet flow is bidirectional

Copy link
Contributor Author

Choose a reason for hiding this comment

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

looking at the ibctest code I see the following

func (commander) CreateChannel(pathName string, opts ibc.CreateChannelOptions, homeDir string) []string {
	return []string{
		"rly", "tx", "channel", pathName,
		"--src-port", opts.SourcePortName,
		"--dst-port", opts.DestPortName,
		"--order", opts.Order.String(),
		"--version", opts.Version,
		"--home", homeDir,
	}
}

It looks like the field names are mappings to the go relayer arguments for channel creation so in this case I think the name makes sense.

I definitely agree that overall we shouldn't use src/dst for our chain names.

}
}
83 changes: 83 additions & 0 deletions e2e/testconfig/testconfig.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package testconfig

import (
"fmt"
"os"

"github.com/strangelove-ventures/ibctest/ibc"
)

const (
DefaultSimdImage = "ghcr.io/cosmos/ibc-go-simd-e2e"
SimdImageEnv = "SIMD_IMAGE"
SimdTagEnv = "SIMD_TAG"
)

// TestConfig holds various fields used in the E2E tests.
type TestConfig struct {
SimdImage string
SimdTag string
}

// FromEnv returns a TestConfig constructed from environment variables.
func FromEnv() TestConfig {
simdImage, ok := os.LookupEnv(SimdImageEnv)
if !ok {
simdImage = DefaultSimdImage
}

simdTag, ok := os.LookupEnv(SimdTagEnv)
if !ok {
panic(fmt.Sprintf("must specify simd version for test with environment variable [%s]", SimdTagEnv))
}

return TestConfig{
SimdImage: simdImage,
SimdTag: simdTag,
}
}

// ChainOptions stores chain configurations for the chains that will be
// created for the tests. They can be modified by passing ChainOptionConfiguration
// to E2ETestSuite.GetChains.
type ChainOptions struct {
ChainAConfig *ibc.ChainConfig
ChainBConfig *ibc.ChainConfig
}

// ChainOptionConfiguration enables arbitrary configuration of ChainOptions.
type ChainOptionConfiguration func(options *ChainOptions)

// DefaultChainOptions returns the default configuration for the chains.
// These options can be configured by passing configuration functions to E2ETestSuite.GetChains.
func DefaultChainOptions() ChainOptions {
tc := FromEnv()
chainACfg := newDefaultSimappConfig(tc, "simapp-a", "chain-a", "atoma")
chainBCfg := newDefaultSimappConfig(tc, "simapp-b", "chain-b", "atomb")
return ChainOptions{
ChainAConfig: &chainACfg,
ChainBConfig: &chainBCfg,
}
}

// newDefaultSimappConfig creates an ibc configuration for simd.
func newDefaultSimappConfig(tc TestConfig, name, chainID, denom string) ibc.ChainConfig {
return ibc.ChainConfig{
Type: "cosmos",
Name: name,
ChainID: chainID,
Images: []ibc.DockerImage{
{
Repository: tc.SimdImage,
Version: tc.SimdTag,
},
},
Bin: "simd",
Bech32Prefix: "cosmos",
Denom: denom,
GasPrices: fmt.Sprintf("0.01%s", denom),
GasAdjustment: 1.3,
TrustingPeriod: "508h",
NoHostMount: false,
}
}
22 changes: 22 additions & 0 deletions e2e/testsuite/relayer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package testsuite

import (
"testing"

dockerclient "github.com/docker/docker/client"
"github.com/strangelove-ventures/ibctest"
"github.com/strangelove-ventures/ibctest/ibc"
"github.com/strangelove-ventures/ibctest/relayer"
"go.uber.org/zap"
)

const (
cosmosRelayerRepository = "ghcr.io/cosmos/relayer"
)

// newCosmosRelayer returns an instance of the go relayer.
func newCosmosRelayer(t *testing.T, logger *zap.Logger, dockerClient *dockerclient.Client, network string, home string) ibc.Relayer {
return ibctest.NewBuiltinRelayerFactory(ibc.CosmosRly, logger, relayer.CustomDockerImage(cosmosRelayerRepository, "main")).Build(
t, dockerClient, network, home,
)
}
Loading