Problem: integration tests CI spend a lot of time #1271
Conversation
Solution: - run in parallel using markers and job matrix
yihuang
requested review from
mmsqe and
leejw51crypto
and removed request for
a team
WalkthroughThe changes represent a significant evolution in the testing infrastructure, introducing a more organized and selective approach to running integration tests. The addition of test markers and the utilization of a matrix strategy allows for more granular control over test execution, enhancing the overall testing process. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ? TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (12)
- .github/workflows/test.yml (1 hunks)
- Makefile (1 hunks)
- integration_tests/conftest.py (1 hunks)
- integration_tests/test_ibc.py (1 hunks)
- integration_tests/test_ibc_rly.py (1 hunks)
- integration_tests/test_ibc_rly_gas.py (1 hunks)
- integration_tests/test_ibc_timeout.py (1 hunks)
- integration_tests/test_ibc_update_client.py (1 hunks)
- integration_tests/test_ica.py (1 hunks)
- integration_tests/test_ica_precompile.py (1 hunks)
- integration_tests/test_upgrade.py (1 hunks)
- scripts/run-integration-tests (1 hunks)
Files skipped from review due to trivial changes (3)
- integration_tests/test_ica.py
- integration_tests/test_ica_precompile.py
- integration_tests/test_upgrade.py
Additional comments: 9
.github/workflows/test.yml (1)
- 20-24: The addition of a matrix strategy for parallel test execution is a good improvement. Ensure that the corresponding job steps are correctly configured to utilize the
TESTS_TO_RUNenvironment variable set by the matrix.Makefile (1)
- 222-226: The
TESTS_TO_RUNvariable has been introduced correctly. It's important to ensure that the scripts and commands that depend on this variable are updated to handle the new possible values.integration_tests/conftest.py (1)
- 16-24: The addition of new markers and the logic to automatically add an "unmarked" marker to tests without any markers are correctly implemented. This will help categorize the tests for the parallel execution strategy.
integration_tests/test_ibc.py (1)
- 15-15: The
pytestmarkdecorator has been added with thegovmarker. Verify that this marker is intended for the tests in this file, as it will affect test selection during execution.integration_tests/test_ibc_rly.py (1)
- 31-31: The
pytestmarkdecorator has been added with thegovmarker. Verify that this marker is intended for the tests in this file, as it will affect test selection during execution.integration_tests/test_ibc_rly_gas.py (1)
- 7-7: The
pytestmarkdecorator has been added with thegovmarker. Verify that this marker is intended for the tests in this file, as it will affect test selection during execution.integration_tests/test_ibc_timeout.py (1)
- 12-12: The
pytestmarkdecorator has been added with thegovmarker. Verify that this marker is intended for the tests in this file, as it will affect test selection during execution.integration_tests/test_ibc_update_client.py (1)
- 9-9: The
pytestmarkdecorator has been added with thegovmarker. Verify that this marker is intended for the tests in this file, as it will affect test selection during execution.scripts/run-integration-tests (1)
- 15-23: The script has been updated to conditionally run tests based on the
TESTS_TO_RUNenvironment variable. Ensure that the test execution commands are compatible with the expected values of this variable and that the conditional logic is robust.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1271 +/- ##
===========================================
- Coverage 35.81% 16.02% -19.79%
===========================================
Files 116 80 -36
Lines 10653 6184 -4469
===========================================
- Hits 3815 991 -2824
+ Misses 6462 5114 -1348
+ Partials 376 79 -297 |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (10)
- integration_tests/conftest.py (1 hunks)
- integration_tests/test_basic.py (1 hunks)
- integration_tests/test_broadcast.py (1 hunks)
- integration_tests/test_gov_update_params.py (1 hunks)
- integration_tests/test_ibc.py (1 hunks)
- integration_tests/test_ibc_rly.py (1 hunks)
- integration_tests/test_ibc_rly_gas.py (1 hunks)
- integration_tests/test_ibc_timeout.py (1 hunks)
- integration_tests/test_ibc_update_client.py (1 hunks)
- scripts/run-integration-tests (2 hunks)
Files skipped from review due to trivial changes (2)
- integration_tests/test_basic.py
- integration_tests/test_ibc.py
Files skipped from review as they are similar to previous changes (4)
- integration_tests/test_ibc_rly.py
- integration_tests/test_ibc_timeout.py
- integration_tests/test_ibc_update_client.py
- scripts/run-integration-tests
Additional comments: 6
integration_tests/conftest.py (2)
13-22: The addition of new markers for categorizing tests is in line with the PR's objectives to enable parallel execution of integration tests. Ensure that the existing test suite is compatible with these new markers and that no tests are unintentionally excluded from the test runs.
25-28: The logic to add an "unmarked" marker to test items without any markers is a good practice for ensuring all tests are categorized. However, please verify that this new categorization does not interfere with the behavior of existing tests that may rely on not having any markers.
integration_tests/test_broadcast.py (1)
- 8-8: The addition of the
pytest.mark.govmarker is consistent with the PR's goal to categorize tests for parallel execution. Ensure that the tests marked withgovare correctly configured in the CI pipeline to run in the intended test group.integration_tests/test_gov_update_params.py (2)
2-2: The import of
pytestis necessary for using the pytest framework's features, such as markers.6-6: The addition of the
pytest.mark.govmarker is consistent with the PR's goal to categorize tests for parallel execution. Ensure that the tests marked withgovare correctly configured in the CI pipeline to run in the intended test group.integration_tests/test_ibc_rly_gas.py (1)
- 7-7: The addition of the
pytest.mark.ibc_rly_gasmarker is consistent with the PR's goal to categorize tests for parallel execution. Ensure that the tests marked withibc_rly_gasare correctly configured in the CI pipeline to run in the intended test group.
Signed-off-by: yihuang <[email protected]>
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (9)
- .github/workflows/test.yml (1 hunks)
- integration_tests/conftest.py (1 hunks)
- integration_tests/test_basic.py (3 hunks)
- integration_tests/test_exported_genesis.py (1 hunks)
- integration_tests/test_ibc_update_client.py (1 hunks)
- integration_tests/test_mempool.py (1 hunks)
- integration_tests/test_pruned_node.py (1 hunks)
- integration_tests/test_replay_block.py (2 hunks)
- integration_tests/test_rollback.py (1 hunks)
Files skipped from review due to trivial changes (4)
- integration_tests/test_mempool.py
- integration_tests/test_pruned_node.py
- integration_tests/test_replay_block.py
- integration_tests/test_rollback.py
Files skipped from review as they are similar to previous changes (2)
- integration_tests/conftest.py
- integration_tests/test_ibc_update_client.py
Additional comments: 6
.github/workflows/test.yml (1)
- 20-24: The strategy for running integration tests in parallel using a matrix of test categories and setting the
TESTS_TO_RUNenvironment variable accordingly looks good. Please ensure to verify the parallel execution of tests in the CI environment to confirm that it works as intended.integration_tests/test_basic.py (4)
141-142: The addition of the
@pytest.mark.slowdecorator to thetest_statesyncfunction is appropriate for categorizing the test for parallel execution. Please ensure that the categorization aligns with the intended parallel execution strategy.258-259: The addition of the
@pytest.mark.slowdecorator to thetest_local_statesyncfunction is appropriate for categorizing the test for parallel execution. Please ensure that the categorization aligns with the intended parallel execution strategy.862-863: The addition of the
@pytest.mark.govdecorator to thetest_submit_any_proposalfunction is appropriate for categorizing the test for parallel execution. Please ensure that the categorization aligns with the intended parallel execution strategy.867-868: The addition of the
@pytest.mark.govdecorator to thetest_submit_send_enabledfunction is appropriate for categorizing the test for parallel execution. Please ensure that the categorization aligns with the intended parallel execution strategy.integration_tests/test_exported_genesis.py (1)
- 9-9: The addition of the
pytest.mark.slowdecorator at the module level is appropriate for categorizing all tests in this file for parallel execution. Please ensure that the categorization aligns with the intended parallel execution strategy.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- .github/workflows/test.yml (1 hunks)
- integration_tests/test_basic.py (1 hunks)
- integration_tests/test_gov_update_params.py (1 hunks)
Files skipped from review due to trivial changes (2)
- integration_tests/test_basic.py
- integration_tests/test_gov_update_params.py
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/test.yml
b47e5c1
) * Problem: integration tests CI spend a lot of time Solution: - run in parallel using markers and job matrix * fix matrix * flock is not nesserary * fix marker * break up ibc test cases * separate gov test cases * split out some slow test cases * split more ibc test * Update .github/workflows/test.yml Signed-off-by: yihuang <[email protected]> * fix pytest lint * more balance --------- Signed-off-by: yihuang <[email protected]>
…1277) * Problem: integration tests CI spend a lot of time (#1271) * Problem: integration tests CI spend a lot of time Solution: - run in parallel using markers and job matrix * fix matrix * flock is not nesserary * fix marker * break up ibc test cases * separate gov test cases * split out some slow test cases * split more ibc test * Update .github/workflows/test.yml Signed-off-by: yihuang <[email protected]> * fix pytest lint * more balance --------- Signed-off-by: yihuang <[email protected]> * Update integration_tests/conftest.py Co-authored-by: yihuang <[email protected]> Signed-off-by: mmsqe <[email protected]> --------- Signed-off-by: yihuang <[email protected]> Signed-off-by: mmsqe <[email protected]> Co-authored-by: yihuang <[email protected]>
Solution:
👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻
PR Checklist:
make)make test)go fmt)golangci-lint run)go list -json -m all | nancy sleuth)Thank you for your code, it's appreciated! :)
Summary by CodeRabbit
New Features
Tests
Chores