-
Notifications
You must be signed in to change notification settings - Fork 616
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
Enable Parallel Transfer Test Suite #6721
Changes from 206 commits
c07bca9
e66bd89
034f472
4cc6a85
71f830c
28ff9b6
4e55137
ca056cf
147cf17
9bbfa1a
4f57916
e04047e
c73d5f6
03cc2e5
65f4476
db85a29
999ed80
be72180
4c333c5
0478cb9
37f5b5e
3fd3345
24e2407
7858528
55d7bf9
1a95262
44e15cb
334d5a9
7466b19
5c46d79
99e6b0a
a114af6
d800caa
dd873a1
a292a3f
e25ba0c
f39d173
9b39944
06ca9a5
7897ef3
786a4f1
5747756
7e2e6df
a84b0e7
43877df
8eae033
dbcff45
bb69698
f19a145
8f86dda
d4b06c8
a9391a4
575403e
50ccd94
87eb32e
e8b9d5a
57aab01
2519593
3bf6c04
64c2fc4
7cd7260
6673e74
550eeef
4c54a88
debc4cb
e50fa40
6d87d95
7c8f516
feb7b01
6fd1773
efcfa5d
21dbb37
70d7a29
465b16c
ec472cf
e483b9a
0c9f368
413b7c1
ae2046a
ba7c4a8
6b3b5aa
5070e50
d874b54
a8cd302
fbb9cd8
c4987a7
a90b671
c348732
789b09b
c00ddd9
fc6c111
72714b3
c546f69
60a6b99
dc47641
832b1bd
360b2f8
cd45c15
84c7c33
007dee1
14d5486
a28a549
d6db0c5
64bc502
1ca1f2f
755b38e
87d1f91
ef6d22f
da2f9f6
e68143b
6619821
43eceed
5fa4fae
aa860ac
8f74d69
5b7cc28
66adec1
2e969c8
bd8fbe1
129fbf8
6f68e9c
f0e6ee2
2b4d24b
57baa91
23ee519
59e3df7
e3ffcff
523c1ae
d39a795
694636c
9c5ae03
6a4d128
c2a6bc6
c3ccbfc
5345776
74088ba
272c12b
4349a1d
24cd07f
dca8c42
37d7335
6e1a082
52c2440
8f9691f
c505243
d715455
4e61ea9
3e2a5fa
5ea9d79
100ccc7
13d3ac6
b6d77d7
4d19be1
2838aef
7afb7f6
0221c6b
cc46a63
e67aeaf
966a644
e92e086
6ea614f
18d0567
1b7214d
1f5e61f
0a78bde
07036a9
1442874
25c73c4
ea3f84c
1e803dc
f17de69
baf3fd3
7d2a881
4ea36c2
0a28e9b
7411f2d
199c14a
c7af85a
25239a2
90d57db
e310e27
f28e937
50daed0
166c72d
400331b
2eca08f
3e53dc3
ddaa142
afc6173
1041e62
912b6cb
627de9c
8162af4
c08db61
18eca24
7091425
441ff6d
8dd2a0a
4cdc4df
e70f54e
be9d9bb
b06dbf2
20c59cd
1a69474
ba91c33
e7d0520
48482c1
d7fa8dd
21c2711
61c1a81
c9949d9
2ec8c1a
6f195b1
d9a7093
c7499fd
e0f9257
cf54298
1a385b6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,11 @@ on: | |
required: false | ||
type: string | ||
default: '' # empty string means run all tests | ||
temp-run-full-suite: | ||
description: 'This flag exists to run a hard coded set of tests and will be phased out' | ||
required: false | ||
type: boolean | ||
default: false | ||
test: | ||
description: 'test name to run as standalone' | ||
required: false | ||
|
@@ -253,3 +258,49 @@ jobs: | |
name: '${{ matrix.entrypoint }}-${{ matrix.test }}' | ||
path: e2e/diagnostics | ||
retention-days: 5 | ||
|
||
e2e-test-suites: | ||
# temporary flag. eventually this field will not exist and this will be the default. | ||
if: ${{ inputs.temp-run-full-suite }} | ||
runs-on: ubuntu-latest | ||
needs: | ||
- build-test-matrix | ||
- docker-build | ||
- docker-build-wasm | ||
env: | ||
CHAIN_IMAGE: '${{ inputs.chain-image }}' | ||
CHAIN_A_TAG: '${{ inputs.chain-a-tag }}' | ||
CHAIN_B_TAG: '${{ inputs.chain-b-tag }}' | ||
RELAYER_IMAGE: '${{ inputs.relayer-image }}' | ||
RELAYER_TAG: '${{ inputs.relayer-tag }}' | ||
RELAYER_ID: '${{ inputs.relayer-type }}' | ||
CHAIN_BINARY: '${{ inputs.chain-binary }}' | ||
CHAIN_UPGRADE_TAG: '${{ inputs.chain-upgrade-tag }}' | ||
CHAIN_UPGRADE_PLAN: '${{ inputs.upgrade-plan-name }}' | ||
strategy: | ||
fail-fast: false | ||
matrix: | ||
include: | ||
# for now we explicitly specify this test suite. | ||
- entrypoint: TestTransferTestSuite | ||
steps: | ||
- uses: actions/checkout@v4 | ||
with: | ||
repository: cosmos/ibc-go | ||
- uses: actions/setup-go@v5 | ||
with: | ||
go-version: '1.22' | ||
cache-dependency-path: 'e2e/go.sum' | ||
- name: Run e2e Test | ||
id: e2e_test | ||
run: | | ||
cd e2e | ||
make e2e-suite entrypoint=${{ matrix.entrypoint }} | ||
Comment on lines
+297
to
+298
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is a copy paste of our existing workflow that just tweaks the make target. |
||
- name: Upload Diagnostics | ||
uses: actions/upload-artifact@v4 | ||
if: ${{ failure() && inputs.upload-logs }} | ||
continue-on-error: true | ||
with: | ||
name: '${{ matrix.entrypoint }}-${{ matrix.test }}' | ||
path: e2e/diagnostics | ||
retention-days: 5 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,4 +63,6 @@ jobs: | |
chain-b-tag: '${{ needs.determine-image-tag.outputs.simd-tag }}' | ||
chain-binary: 'simd' | ||
# on regular PRs we won't run upgrade tests. | ||
test-exclusions: 'TestUpgradeTestSuite,TestGrandpaTestSuite,TestIBCWasmUpgradeTestSuite' | ||
# NOTE: we are exluding TestTransferTestSuite as we run this full suite instead of each individual test. | ||
test-exclusions: 'TestUpgradeTestSuite,TestGrandpaTestSuite,TestIBCWasmUpgradeTestSuite,TestTransferTestSuite' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we exclude |
||
temp-run-full-suite: true |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,8 +2,8 @@ | |
|
||
set -eo pipefail | ||
|
||
TEST="${1}" | ||
ENTRY_POINT="${2:-}" | ||
TEST="${1:-}" | ||
export ENTRY_POINT="${2:-}" | ||
|
||
function _verify_jq() { | ||
if ! command -v jq > /dev/null ; then | ||
|
@@ -19,7 +19,7 @@ function _verify_fzf() { | |
fi | ||
} | ||
|
||
function _verify_dependencies() { | ||
function _verify_test_dependencies() { | ||
if [ -z "${TEST}" ]; then | ||
# fzf is only required if we are not explicitly specifying a test. | ||
_verify_fzf | ||
|
@@ -28,6 +28,14 @@ function _verify_dependencies() { | |
_verify_jq | ||
} | ||
|
||
function _verify_suite_dependencies() { | ||
if [ -z "${ENTRY_POINT}" ]; then | ||
# fzf is only required if we are not explicitly specifying an entrypoint. | ||
_verify_fzf | ||
fi | ||
# jq is always required to determine the entrypoint of the test. | ||
_verify_jq | ||
} | ||
|
||
# _select_test_config lets you dynamically select a test config for the specific test. | ||
function _select_test_config() { | ||
|
@@ -51,7 +59,7 @@ function _get_test(){ | |
|
||
# run_test runs a single E2E test. | ||
function run_test() { | ||
# if the dev configs directory is present, enable fzf completion to select a test config file to use. | ||
_verify_test_dependencies | ||
|
||
# if test is set, that is used directly, otherwise the test can be interactively provided if fzf is installed. | ||
TEST="$(_get_test ${TEST})" | ||
|
@@ -78,23 +86,23 @@ function run_test() { | |
|
||
# run_suite runs a full E2E test suite. | ||
function run_suite() { | ||
_verify_suite_dependencies | ||
# if jq is installed, we can automatically determine the test entrypoint. | ||
if command -v jq > /dev/null; then | ||
if [ -z "${ENTRY_POINT}" ]; then | ||
cd .. | ||
ENTRY_POINT="$(go run -mod=readonly cmd/build_test_matrix/main.go | jq -r '.include[] | .entrypoint' | uniq | fzf)" | ||
cd - > /dev/null | ||
fi | ||
|
||
# find the name of the file that has this test in it. | ||
test_file="$(grep --recursive --files-with-matches './tests' -e "${ENTRY_POINT}")" | ||
test_file="$(grep --recursive --files-with-matches './tests' -e "${ENTRY_POINT}(")" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [q] why the added extra parentheses? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If multiple tests start with
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, it makes sense! |
||
test_dir="$(dirname $test_file)" | ||
|
||
# TODO: add the -p flag to run tests in parallel | ||
go test -v "${test_dir}" --run ${ENTRY_POINT} -timeout 30m | ||
go test -v "${test_dir}" --run ^${ENTRY_POINT}$ -timeout 30m -p 10 | ||
} | ||
|
||
_verify_dependencies | ||
|
||
# if the dev configs directory is present, enable fzf completion to select a test config file to use. | ||
if [[ -d "dev-configs" ]]; then | ||
export E2E_CONFIG_PATH="$(pwd)/dev-configs/$(_select_test_config)" | ||
echo "Using configuration file at ${E2E_CONFIG_PATH}" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,8 +33,8 @@ type ConnectionTestSuite struct { | |
testsuite.E2ETestSuite | ||
} | ||
|
||
func (s *ConnectionTestSuite) SetupTest() { | ||
s.SetupPaths(ibc.DefaultClientOpts(), s.TransferChannelOptions()) | ||
func (s *ConnectionTestSuite) SetupConnectionTestPath(testName string) (ibc.Relayer, ibc.ChannelOutput) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's no longer a lifecycle method, but one we call at the start of tests. the same thing applies for every change of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd love if the name of the method made it clearer what this function is returning, but naming in hard :) |
||
return s.SetupPaths(ibc.DefaultClientOpts(), s.TransferChannelOptions(), testName), s.GetChainAChannelForTest(testName) | ||
} | ||
|
||
// QueryMaxExpectedTimePerBlockParam queries the on-chain max expected time per block param for 03-connection | ||
|
@@ -65,7 +65,8 @@ func (s *ConnectionTestSuite) TestMaxExpectedTimePerBlockParam() { | |
t := s.T() | ||
ctx := context.TODO() | ||
|
||
relayer, channelA := s.GetRelayer(), s.GetChainAChannel() | ||
testName := t.Name() | ||
relayer, channelA := s.SetupConnectionTestPath(testName) | ||
|
||
chainA, chainB := s.GetChains() | ||
chainAVersion := chainA.Config().Images[0].Version | ||
|
@@ -126,7 +127,7 @@ func (s *ConnectionTestSuite) TestMaxExpectedTimePerBlockParam() { | |
}) | ||
|
||
t.Run("start relayer", func(t *testing.T) { | ||
s.StartRelayer(relayer) | ||
s.StartRelayer(relayer, testName) | ||
}) | ||
|
||
t.Run("packets are relayed", func(t *testing.T) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can hard code these in as we transition, and then replace this with the dynamically generated list of tests.