fix: Unable to build ibctest binary #326
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DavidNix
commented
Oct 14, 2022
| @@ -10,7 +10,7 @@ ibctest: gen ## Build ibctest binary into ./bin | |||
|
|
|||
| .PHONY: test | |||
| test: ## Run unit tests | |||
| @go test -cover -short -race -timeout=60s $(shell go list ./... | grep -v /cmd/) | |||
| @go test -cover -short -race -timeout=60s ./... | |||
There was a problem hiding this comment.
Proper use of testing.Short() and -short flag means we can run unit tests throughout the whole project. No need to filter out packages.
jtieri
approved these changes
Oct 14, 2022
| test-cmd-ibctest: | ||
| name: test-cmd-ibctest | ||
| # -short flag purposefully omitted because there are some longer unit tests | ||
| run: go test -race -timeout 10m -p 2 $(go list ./... | grep -v /cmd | grep -v /examples) |
There was a problem hiding this comment.
It seems like we are expecting some longer running tests here but the timeout value changes from 30m to 10m, does that make sense or should we use the larger value still?
There was a problem hiding this comment.
I'm hoping there are no large interchain tests in this set of tests which is why I moved it down to 10m. That could prove wrong, though.
We have a couple unit tests that are skipped with -short.
jtieri
pushed a commit
that referenced
this pull request
Jan 6, 2023
jtieri
pushed a commit
that referenced
this pull request
Jan 6, 2023
jtieri
added a commit
that referenced
this pull request
Jan 9, 2023
* fix: Unable to build ibctest binary (#326) * feat: Add cosmos message poller (#325) * fix: change import paths from v6 to v3 * Make configutil public (#336) * Make configutil public * Rename test package to chainutil. Make util packages subpackages of util package * Consolidate into single testutil package * fix: formatting issue in doc * fix: change import path from test to testutil * fix: go mod tidy * packet-forward-middleware packet memo, retry on timeout, and atomic forwards (#306) * Make examples its own module * Make cosmos specific module and run in CI in separate task * Update e2e test for memo refactor * Update test to chain-level params * Use gaia with pfm with configurable timeout and retries * Update SendIBCTransfer uses * fix interchain test * Add GetClients method to Relayer and helper for getting transfer channel between chains * Update packet forward test for multi-hop, add multi-hop refund test * Update tests for atomic forwards * Wait for a block after ack before checking balances * reduce wait to 1 block * Add multi-hop flow with refund through chain with native denom. Add assertions for escrow accounts * Remove stale comment * handle feedback * Add TransferOptions * fix: remove extra argument to NewMsgTransfer call * build: go mod tidy * chore: update gaia image version for packet forward tests * fix: fallback to old version of the interchain accounts demo binary Co-authored-by: David Nix <[email protected]> Co-authored-by: Andrew Gouin <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With a
go.modin dircmd/ibctest, we are unable to build theibctestartifact:It's also uncommon to have a nested
go.modwithin thecmddirectory.I was able to preserve the test partitioning and eliminate all the extra nested
go.mod. Simply run the tests in a specific dir. (i.e. don't do./...).Use testing.Short()
I've taken the strategy that all e2e or integration tests must skip the test if
testing.Short()is true. To me, this makes sense because conformance and other e2e tests may take a long time, > 1 minute. They are anything but short.This will take developer discipline but the unit tests via
make testwill timeout after 60s. Therefore, that should catch where we miss atesting.Short()check.