test: make #18472 test structure same as backports #18504
Conversation
WalkthroughThe changes involve minor updates and refactoring across several files in a codebase, likely related to a blockchain or financial software project. A test package is renamed, global variables are added for testing, and a new test case is introduced. Additionally, there's a documentation update in the changelog and a clarifying comment in a configuration-related file. These changes seem to be part of routine maintenance and improvement of the code and its documentation. Changes
Poem
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 (3)
- client/tx/aux_builder_test.go (4 hunks)
- client/tx/tx_test.go (1 hunks)
- tools/confix/CHANGELOG.md (1 hunks)
Files skipped from review due to trivial changes (1)
- client/tx/tx_test.go
Additional comments: 5
tools/confix/CHANGELOG.md (1)
- 34-34: The changelog update correctly references the pull request and provides a brief description of the change. It follows the established format and is placed under the "Unreleased" section, which is appropriate for changes that have not yet been included in a released version.
client/tx/aux_builder_test.go (4)
1-11: The renaming of the package from
txtotx_testis a good practice to avoid import cycles and to indicate that the file contains tests for thetxpackage. This change also helps to ensure that the test code is only compiled when running tests and not included in the final build of the package.15-32: The introduction of new global variables (
memo,timeoutHeight,rawSig,msg1, andchainID) is a common pattern in tests to define constants and variables that will be reused across different test cases. However, it's important to ensure that these variables are not mutated during test execution to prevent side effects that could affect other tests. If mutation is necessary, consider using a setup function to provide a fresh set of variables for each test case.40-46: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [34-217]
The test cases are well-structured and cover a variety of scenarios, including error cases and successful paths. The use of table-driven tests is a good practice for clarity and maintainability. However, it's important to reset the
AuxTxBuilderbefore each test case to avoid state leakage between tests. This is correctly done on line 217 withb = tx.NewAuxTxBuilder(), ensuring that each test case starts with a fresh builder.
- 214-220: The loop correctly creates a new scope for each test case, preventing parallel test execution issues. However, if tests are to be run in parallel, you would need to add
t.Parallel()inside thet.Runfunction. Since this is not present, it's assumed that parallel execution is not intended or necessary for these tests.
Description
Small follow-up of #18472
When backporting this fix, the package name changes of tests was giving an import cycle in the backport PR.
As I had to modify some files in both backport, I make those changes as well here to keep consistency and reduce the diff we'll have during our audit.
ref: #18502
ref: #18503
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.mdmake lintandmake testReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!in the type prefix if API or client breaking changeSummary by CodeRabbit
Refactor
Tests
Documentation
Chores