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

Jump to bottom

test: fix e2e tests broken in various ways #2078

Merged
merged 5 commits into from
Jul 24, 2024
Merged

test: fix e2e tests broken in various ways #2078

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ebc9290
Fix failing compatiblity tests
bermuell Jul 22, 2024
7e05a2f
Improve identification of closest ICS version for a git reference
bermuell Jul 22, 2024
3ecc5d7
fix e2e tests
bermuell Jul 22, 2024
ef557b1
Fix e2e query command for rewards
bermuell Jul 22, 2024
f60b7ca
Update tests/e2e/config.go
bermuell Jul 24, 2024
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
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ vendor/
build/
.vscode
.idea
__debug_*

# docusaurus versioned docs created during build
docs/versioned_docs
Expand Down
24 changes: 12 additions & 12 deletions tests/e2e/actions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1000,10 +1000,10 @@ func (tr Chain) addChainToGorelayer(
}

addChainCommand := tr.target.ExecCommand("rly", "chains", "add", "--file", chainConfigFileName, string(ChainId))
e2e.ExecuteCommand(addChainCommand, "add chain")
e2e.ExecuteCommand(addChainCommand, "add chain", verbose)

keyRestoreCommand := tr.target.ExecCommand("rly", "keys", "restore", string(ChainId), "default", tr.testConfig.validatorConfigs[action.Validator].Mnemonic)
e2e.ExecuteCommand(keyRestoreCommand, "restore keys")
e2e.ExecuteCommand(keyRestoreCommand, "restore keys", verbose)
}

func (tr Chain) addChainToHermes(
Expand Down Expand Up @@ -1109,7 +1109,7 @@ func (tr Chain) addIbcConnectionGorelayer(

//#nosec G204 -- Bypass linter warning for spawning subprocess with cmd arguments.
pathConfigCommand := tr.target.ExecCommand("bash", "-c", bashCommand)
e2e.ExecuteCommand(pathConfigCommand, "add path config")
e2e.ExecuteCommand(pathConfigCommand, "add path config", verbose)

//#nosec G204 -- Bypass linter warning for spawning subprocess with cmd arguments.
newPathCommand := tr.target.ExecCommand("rly",
Expand All @@ -1120,20 +1120,20 @@ func (tr Chain) addIbcConnectionGorelayer(
"--file", pathConfigFileName,
)

e2e.ExecuteCommand(newPathCommand, "new path")
e2e.ExecuteCommand(newPathCommand, "new path", verbose)

//#nosec G204 -- Bypass linter warning for spawning subprocess with cmd arguments.
newClientsCommand := tr.target.ExecCommand("rly", "transact", "clients", pathName)

e2e.ExecuteCommand(newClientsCommand, "new clients")
e2e.ExecuteCommand(newClientsCommand, "new clients", verbose)

tr.waitBlocks(action.ChainA, 1, 10*time.Second)
tr.waitBlocks(action.ChainB, 1, 10*time.Second)

//#nosec G204 -- Bypass linter warning for spawning subprocess with cmd arguments.
newConnectionCommand := tr.target.ExecCommand("rly", "transact", "connection", pathName)

e2e.ExecuteCommand(newConnectionCommand, "new connection")
e2e.ExecuteCommand(newConnectionCommand, "new connection", verbose)

tr.waitBlocks(action.ChainA, 1, 10*time.Second)
tr.waitBlocks(action.ChainB, 1, 10*time.Second)
Expand Down Expand Up @@ -1300,7 +1300,7 @@ func (tr Chain) addIbcChannelGorelayer(
"--order", action.Order,
"--debug",
)
e2e.ExecuteCommand(cmd, "addChannel")
e2e.ExecuteCommand(cmd, "addChannel", verbose)
}

func (tr Chain) addIbcChannelHermes(
Expand Down Expand Up @@ -1373,16 +1373,16 @@ func (tr Chain) transferChannelComplete(
log.Fatal("transferChannelComplete is not implemented for rly")
}

chanOpenTryCmd := tr.target.ExecCommand("hermes",
"tx", "chan-open-try",
chanOpenTryCmd := tr.target.ExecCommand("hermes", "tx", "chan-open-try",
"--dst-chain", string(tr.testConfig.chainConfigs[action.ChainB].ChainId),
"--src-chain", string(tr.testConfig.chainConfigs[action.ChainA].ChainId),
"--dst-connection", "connection-"+fmt.Sprint(action.ConnectionA),
"--dst-port", action.PortB,
"--src-port", action.PortA,
"--src-channel", "channel-"+fmt.Sprint(action.ChannelA),
)
e2e.ExecuteCommand(chanOpenTryCmd, "transferChanOpenTry")

e2e.ExecuteCommand(chanOpenTryCmd, "transferChanOpenTry", verbose)

chanOpenAckCmd := tr.target.ExecCommand("hermes",
"tx", "chan-open-ack",
Expand All @@ -1395,7 +1395,7 @@ func (tr Chain) transferChannelComplete(
"--src-channel", "channel-"+fmt.Sprint(action.ChannelB),
)

e2e.ExecuteCommand(chanOpenAckCmd, "transferChanOpenAck")
e2e.ExecuteCommand(chanOpenAckCmd, "transferChanOpenAck", verbose)

chanOpenConfirmCmd := tr.target.ExecCommand("hermes",
"tx", "chan-open-confirm",
Expand All @@ -1407,7 +1407,7 @@ func (tr Chain) transferChannelComplete(
"--dst-channel", "channel-"+fmt.Sprint(action.ChannelB),
"--src-channel", "channel-"+fmt.Sprint(action.ChannelA),
)
e2e.ExecuteCommand(chanOpenConfirmCmd, "transferChanOpenConfirm")
e2e.ExecuteCommand(chanOpenConfirmCmd, "transferChanOpenConfirm", verbose)
}

type RelayPacketsAction struct {
Expand Down
13 changes: 11 additions & 2 deletions tests/e2e/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,15 +128,24 @@ func (tr *TestConfig) Initialize() {
// Note: if no matching version is found an empty string is returned
func getIcsVersion(reference string) string {
icsVersion := ""
if reference == "" {

if reference == "" || reference == VLatest {
return icsVersion
}

if semver.IsValid(reference) {
// remove build suffix
return semver.Canonical(reference)
}

for _, tag := range []string{"v2.0.0", "v2.4.0", "v2.4.0-lsm", "v3.1.0", "v3.2.0", "v3.3.0", "v4.0.0", "v4.1.1", "v4.1.1-lsm"} {
// List of all tags matching vX.Y.Z or vX.Y.X-lsm in ascending order
bermuell marked this conversation as resolved.
Show resolved Hide resolved
cmd := exec.Command("git", "tag", "-l", "--sort", "v:refname", "v*.?", "v*.?-lsm", "v*.??", "v*.??-lsm")
out, err := cmd.CombinedOutput()
if err != nil {
panic(fmt.Sprintf("Error getting sorted tag list from git: %s", err.Error()))
}
Comment on lines +142 to +146
Copy link
Contributor

Choose a reason for hiding this comment

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

Handle potential security risks with exec.Command.

The use of exec.Command to execute Git commands introduces potential security risks. Ensure that the inputs to exec.Command are sanitized and consider using a safer alternative if possible.

- cmd := exec.Command("git", "tag", "-l", "--sort", "v:refname", "v*.?", "v*.?-lsm", "v*.??", "v*.??-lsm")
+ cmd := exec.Command("git", "tag", "-l", "--sort", "v:refname", "v*.?", "v*.?-lsm", "v*.??", "v*.??-lsm")
+ cmd.Env = append(os.Environ(), "GIT_TERMINAL_PROMPT=0")

Committable suggestion was skipped due to low confidence.

icsVersions := strings.Split(string(out), "\n")
for _, tag := range icsVersions {
Comment on lines +147 to +148
Copy link
Contributor

Choose a reason for hiding this comment

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

Improve error handling for Git command execution.

The function currently panics if the Git command fails. Consider improving error handling by logging the error and returning a more descriptive error message.

- if err != nil {
-   panic(fmt.Sprintf("Error getting sorted tag list from git: %s", err.Error()))
+ if err != nil {
+   log.Printf("Error getting sorted tag list from git: %s", err.Error())
+   return ""
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
icsVersions := strings.Split(string(out), "\n")
for _, tag := range icsVersions {
icsVersions := strings.Split(string(out), "\n")
for _, tag := range icsVersions {
if err != nil {
log.Printf("Error getting sorted tag list from git: %s", err.Error())
return ""
}

//#nosec G204 -- Bypass linter warning for spawning subprocess with cmd arguments
cmd := exec.Command("git", "merge-base", "--is-ancestor", reference, tag)
out, err := cmd.CombinedOutput()
Expand Down
7 changes: 4 additions & 3 deletions tests/e2e/state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ func (tr Chain) curlJsonRPCRequest(method, params, address string) {
cmd := tr.target.ExecCommand("bash", "-c", fmt.Sprintf(cmd_template, method, params, address))

verbosity := false
e2e.ExecuteCommandWithVerbosity(cmd, "curlJsonRPCRequest", verbosity)
e2e.ExecuteCommand(cmd, "curlJsonRPCRequest", verbosity)
}

func uintPtr(i uint) *uint {
Expand Down Expand Up @@ -357,8 +357,8 @@ func (tr Commands) GetReward(chain ChainID, validator ValidatorID, blockHeight u

binaryName := tr.chainConfigs[chain].BinaryName
cmd := tr.target.ExecCommand(binaryName,
"query", "distribution", "delegation-total-rewards",
"--delegator-address", delAddresss,
"query", "distribution", "rewards",
delAddresss,
`--height`, fmt.Sprint(blockHeight),
`--node`, tr.GetQueryNode(chain),
`-o`, `json`,
Expand All @@ -367,6 +367,7 @@ func (tr Commands) GetReward(chain ChainID, validator ValidatorID, blockHeight u
bz, err := cmd.CombinedOutput()

if err != nil {
log.Println("running cmd: ", cmd)
log.Fatal("failed getting rewards: ", err, "\n", string(bz))
}

Expand Down
3 changes: 2 additions & 1 deletion tests/e2e/steps_compatibility.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

gov "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
providertypes "github.com/cosmos/interchain-security/v5/x/ccv/provider/types"
)

func compstepStartProviderChain() []Step {
Expand Down Expand Up @@ -98,7 +99,7 @@ func compstepsStartConsumerChain(consumerName string, proposalIndex, chainIndex
ConsumerPubkey: getDefaultValidators()[ValidatorID("carol")].ConsumerValPubKey,
ReconfigureNode: false,
ExpectError: true,
ExpectedError: "a validator has assigned the consumer key already: consumer key is already in use by a validator",
ExpectedError: providertypes.ErrConsumerKeyInUse.Error(),
},
State: State{
ChainID(consumerName): ChainState{
Expand Down
9 changes: 1 addition & 8 deletions tests/e2e/testlib/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ import (
"os/exec"
)

var verbose *bool //TODO: remove hack

func ExecuteCommandWithVerbosity(cmd *exec.Cmd, cmdName string, verbose bool) {
func ExecuteCommand(cmd *exec.Cmd, cmdName string, verbose bool) {
if verbose {
fmt.Println(cmdName+" cmd:", cmd.String())
}
Expand All @@ -36,8 +34,3 @@ func ExecuteCommandWithVerbosity(cmd *exec.Cmd, cmdName string, verbose bool) {
log.Fatal(err)
}
}

// Executes a command with verbosity specified by CLI flag
func ExecuteCommand(cmd *exec.Cmd, cmdName string) {
ExecuteCommandWithVerbosity(cmd, cmdName, *verbose)
}
2 changes: 1 addition & 1 deletion tests/e2e/v4/state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ func (tr Commands) curlJsonRPCRequest(method, params, address string) {
cmd := tr.Target.ExecCommand("bash", "-c", fmt.Sprintf(cmd_template, method, params, address))

verbosity := false
e2e.ExecuteCommandWithVerbosity(cmd, "curlJsonRPCRequest", verbosity)
e2e.ExecuteCommand(cmd, "curlJsonRPCRequest", verbosity)
}

// GetClientFrozenHeight returns the frozen height for a client with the given client ID
Expand Down
Loading