-
Notifications
You must be signed in to change notification settings - Fork 41
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
Fix the rest of the tests #1993
Conversation
… and remove all other ProposalContents methods since we don't have any anymore, and it's no longer required.
…oposalMsg. I'm keeping the others as is since they require more logic than is available with a WeightedProposalMsg, e.g. providing more deposit than is required.
…li tests in the hope that it makes that test randomly fail less often. It still gets the 'panic: leveldb closed' error, though, and there's no way to catch it because it's happening in a separate process, outside anywhere we can put a defer. It's in the peer management stuff. The node starts shutting down, but doesn't kill that process properly before shutting down the db. That sometimes results in it trying to read the db and panicking. It's just a luck-of-the-draw thing. I think the only way to fully prevent it would be to drop down to one or two validators, but we need the 5 for tests in there, so I think we'll just have suffer.
…ightedProposalMsg, and give it the correct authority.
…nerate thing on the updateParams weight, just like we used to do when it was just a normal sim msg.
…ecause a random channel doesn't exist. It didn't look like it was easy (and might even be impossible) to set up a channel for use in the unit tests. So I think this is the best I can do here.
…pdate params stuff.
… Use it for the oracle simulation tests.
…randIntBetween private funcs. Create SelectRandomEntries and use it to get random accounts instead of redefining that functionality in multiple modules.
…r messages and create the SelectRandomAccounts wrapper for it.
…used a little differently when getting just one account.
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 your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
…ake the test workflow finish a minute faster.
…kgs and re-add the corrected string to the corrected part file. I got so much wrong there.
…e the OUT of part 3. Ugh.
…t have a positive effect.
…get the encoding config rather than making yet another tempapp for that.
… that in all the unit tests that has keyring accounts.
…eady has one, and after some testing, it appears to actually make things more likely to accidentally fail.
…ithPower funcs up in the file so they aren't right in the middle of the SimTestHelper stuff.
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.
Actionable comments posted: 6
Outside diff range and nitpick comments (6)
internal/provwasm/simulation.go (2)
Line range hint
130-160
: Consider improving error handling inSimulateMsgBindName
.Replace
panic
with more graceful error handling to prevent simulation crashes and allow for more controlled testing scenarios.
Line range hint
250-270
: Add error handling for file operations inSimulateMsgStoreContract
.Instead of using
panic
for file read errors, handle these errors gracefully to allow for better control and testing of the simulation environment.x/sanction/simulation/operations_test.go (2)
4-4
: Add a brief comment explaining the purpose of importing"fmt"
.It's good practice to document the purpose of imports, especially when they are not directly obvious within the provided context.
231-232
: Clarify the purpose ofWeightedOpsArgs
struct.Consider adding a comment above the
WeightedOpsArgs
struct to explain its role and usage within the test suite. This enhances code readability and maintainability.x/marker/client/cli/cli_test.go (1)
Line range hint
56-70
: Refactor theGenerateAccountsWithKeyrings
method to improve error handling.func (s *IntegrationTestSuite) GenerateAccountsWithKeyrings(number int) { - s.keyringEntries, s.keyring = testutil.GenerateTestKeyring(s.T(), number, s.cfg.Codec) - s.accountAddresses = testutil.GetKeyringEntryAddresses(s.keyringEntries) + keyringEntries, keyring, err := testutil.GenerateTestKeyring(s.T(), number, s.cfg.Codec) + if err != nil { + s.T().Fatal("Failed to generate test keyring:", err) + } + s.keyringEntries = keyringEntries + s.keyring = keyring + accountAddresses, err := testutil.GetKeyringEntryAddresses(s.keyringEntries) + if err != nil { + s.T().Fatal("Failed to get keyring entry addresses:", err) + } + s.accountAddresses = accountAddresses }This change ensures that any errors during the generation of the test keyring or retrieval of addresses are properly handled, preventing silent failures during test setup.
x/metadata/client/cli/cli_test.go (1)
Line range hint
499-510
: Consider using a helper function for generating test keyrings.To improve code readability and maintainability, consider extracting the logic for generating test keyrings into a separate helper function. This will make the
SetupSuite
method cleaner and easier to understand.- s.keyringAccounts, s.keyring = testutil.GenerateTestKeyring(s.T(), number, s.cfg.Codec) + s.keyringAccounts, s.keyring = s.generateTestKeyring(number)And then define the helper method:
func (s *IntegrationCLITestSuite) generateTestKeyring(number int) ([]testutil.TestKeyringEntry, keyring.Keyring) { return testutil.GenerateTestKeyring(s.T(), number, s.cfg.Codec) }
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (42)
- .github/workflows/test.yml (1 hunks)
- app/app.go (2 hunks)
- app/params/weights.go (1 hunks)
- internal/helpers/rand.go (1 hunks)
- internal/helpers/rand_test.go (1 hunks)
- internal/provwasm/simulation.go (2 hunks)
- testutil/keyring.go (1 hunks)
- testutil/network.go (2 hunks)
- testutil/sims.go (1 hunks)
- x/attribute/client/cli/cli_test.go (3 hunks)
- x/attribute/simulation/operations_test.go (2 hunks)
- x/exchange/client/cli/cli_test.go (3 hunks)
- x/ibcratelimit/client/cli/cli_test.go (4 hunks)
- x/ibcratelimit/keeper/keeper.go (1 hunks)
- x/ibcratelimit/module/module.go (4 hunks)
- x/ibcratelimit/simulation/genesis.go (2 hunks)
- x/ibcratelimit/simulation/genesis_test.go (2 hunks)
- x/ibcratelimit/simulation/operations.go (2 hunks)
- x/ibcratelimit/simulation/operations_test.go (3 hunks)
- x/marker/client/cli/cli_test.go (4 hunks)
- x/marker/simulation/operations_test.go (5 hunks)
- x/marker/types/key.go (1 hunks)
- x/metadata/client/cli/cli_test.go (4 hunks)
- x/msgfees/client/cli/cli_test.go (3 hunks)
- x/name/simulation/operations.go (3 hunks)
- x/name/simulation/operations_test.go (2 hunks)
- x/oracle/client/cli/cli_test.go (4 hunks)
- x/oracle/module/module.go (2 hunks)
- x/oracle/simulation/genesis.go (3 hunks)
- x/oracle/simulation/genesis_test.go (2 hunks)
- x/oracle/simulation/operations.go (5 hunks)
- x/oracle/simulation/operations_test.go (3 hunks)
- x/quarantine/simulation/operations_test.go (2 hunks)
- x/sanction/client/testutil/cli_test.go (2 hunks)
- x/sanction/client/testutil/common_test.go (3 hunks)
- x/sanction/module/module.go (2 hunks)
- x/sanction/simulation/operations.go (3 hunks)
- x/sanction/simulation/operations_test.go (17 hunks)
- x/trigger/client/cli/cli_test.go (4 hunks)
- x/trigger/simulation/genesis.go (4 hunks)
- x/trigger/simulation/operations.go (4 hunks)
- x/trigger/simulation/operations_test.go (2 hunks)
Files skipped from review due to trivial changes (3)
- .github/workflows/test.yml
- x/marker/types/key.go
- x/oracle/simulation/genesis_test.go
Additional Context Used
Learnings (1)
x/marker/client/cli/cli_test.go (2)
User: nullpointer0x00 PR: provenance-io/provenance#1953 File: x/msgfees/client/cli/cli_test.go:0-0 Timestamp: 2024-05-14T14:00:46.007Z Learning: The function `GenerateAccountsWithKeyrings` is widely used across different test suites for setting up test environments. However, its error handling is not robustly tested.
User: nullpointer0x00 PR: provenance-io/provenance#1953 File: x/msgfees/client/cli/cli_test.go:0-0 Timestamp: 2024-05-14T14:00:46.007Z Learning: The function `GenerateAccountsWithKeyrings` is widely used across different test suites for setting up test environments. However, its error handling is not robustly tested.
Additional comments not posted (71)
app/params/weights.go (1)
31-31
: The addition ofDefaultWeightIBCRLUpdateParams
with a weight of 100 seems appropriate, reflecting its significance in the operations.x/ibcratelimit/simulation/operations.go (1)
21-29
: The introduction ofProposalMsgs
function aligns well with the module's simulation requirements. Good use ofDefaultWeightIBCRLUpdateParams
to maintain consistency across the module.internal/helpers/rand.go (1)
10-13
: The new helper functionsRandIntBetween
andSelectRandomEntries
are correctly implemented and will be useful for random selections in simulations.Also applies to: 15-38
x/ibcratelimit/simulation/operations_test.go (1)
46-64
: The updates to the test functionsTestProposalMsgs
andTestSimulatePropMsgUpdateOracle
effectively utilize the new test utilities and provide clear, maintainable test setups.Also applies to: 67-78
x/ibcratelimit/keeper/keeper.go (1)
77-80
: The implementation ofGetAuthority
is straightforward and correctly retrieves the authority account address.testutil/keyring.go (3)
15-28
: The newTestKeyringEntry
type andGenerateTestKeyring
function are well-implemented for creating and managing test keyrings.
30-67
: TheNewTestKeyring
andGenerateTestAccountsInKeyring
functions are correctly implemented to support testing scenarios that require multiple keyring entries.
69-79
: TheGetKeyringEntryAddresses
function is a useful utility for retrieving addresses from keyring entries, correctly implemented.x/ibcratelimit/simulation/genesis_test.go (2)
Line range hint
8-36
: The tests forContractFn
are well-structured to validate the function's behavior under different random conditions.
Line range hint
38-74
: TheRandomizedGenState
tests are comprehensive and correctly use randomness to test various scenarios, ensuring the function behaves as expected across different states.x/oracle/simulation/operations_test.go (2)
Line range hint
12-67
: The introduction ofSimTestHelper
and its usage in the oracle simulation tests enhances the clarity and maintainability of the test code.
69-107
: The tests forWeightedOperations
andProposalMsgs
are correctly implemented, effectively using theSimTestHelper
to assert conditions in a streamlined manner.x/sanction/client/testutil/common_test.go (2)
Line range hint
8-83
: The setup suite and the utility functions in theIntegrationTestSuite
are well-constructed, providing robust tools for setting up and manipulating the test environment for the sanction module.
132-148
: The utility functions for logging and waiting for specific blockchain heights are correctly implemented and will be useful for synchronization in integration tests.x/oracle/simulation/operations.go (2)
Line range hint
21-59
: TheWeightedOperations
andProposalMsgs
functions are correctly implemented to generate simulation operations and governance proposals with dynamic weights based on randomness.
Line range hint
61-158
: The simulation functionsSimulatePropMsgUpdateOracle
andSimulateMsgSendQueryOracle
are well-implemented, effectively using randomness to simulate different scenarios and interactions within the oracle module.x/trigger/simulation/operations.go (2)
19-19
: The import ofgithub.com/provenance-io/provenance/internal/helpers
is added to utilize helper functions for random account selection. This is a good practice as it leverages existing utilities to avoid code duplication and potential bugs.
59-59
: The use ofhelpers.SelectRandomAccounts
to select random accounts for the simulation is a good application of the DRY principle, ensuring that random selection logic is centralized and consistent across the application.x/ibcratelimit/module/module.go (2)
32-32
: The implementation of themodule.HasProposalMsgs
interface inAppModule
is a significant change. It ensures that the module can now contribute to governance proposals, aligning with the extensibility and modular design of the Cosmos SDK.
94-94
: Refactoring inAppModule
to remove unnecessary dependencies and streamline the module's structure is a positive change. It simplifies the module's initialization and enhances clarity by focusing on essential components.Also applies to: 98-98, 127-129, 132-133
x/sanction/module/module.go (2)
33-33
: Implementing themodule.HasProposalMsgs
interface in theAppModule
of the sanction module allows it to participate in governance proposals. This is a crucial feature for modules that may need configurable parameters or behaviors.
167-173
: TheProposalMsgs
function implementation in the sanction module is crucial for enabling governance-driven changes. It allows the module to propose changes through governance, which is essential for maintaining flexibility and adaptability in network parameters.x/oracle/module/module.go (2)
36-36
: The oracle module now implements themodule.HasProposalMsgs
interface, which is essential for modules that need to interact with governance mechanisms in blockchain applications.
144-147
: TheProposalMsgs
function in the oracle module enables it to submit governance proposals, which is a critical feature for dynamic parameter updates and operational flexibility.x/ibcratelimit/client/cli/cli_test.go (1)
34-36
: The refactoring in the test suite to better manage keyrings and network configurations is a good improvement. It enhances the readability and maintainability of the test code, making it easier to understand and modify.Also applies to: 37-38, 105-105, 115-116
x/trigger/simulation/operations_test.go (2)
20-20
: The addition ofgithub.com/provenance-io/provenance/testutil
in the imports is appropriate for accessing utility functions that facilitate testing. This helps in maintaining clean and DRY test code.
174-174
: Utilizingtestutil.GenerateTestingAccounts
to create test accounts is a good practice as it centralizes account generation in tests, ensuring consistency and reducing boilerplate code.testutil/network.go (2)
58-58
: Change to usetempApp.GetEncodingConfig()
enhances flexibility by using the app's specific configuration.
58-58
: Removal of delay inCleanup
function should speed up tests, assuming it's correctly implemented as described.x/quarantine/simulation/operations_test.go (1)
32-32
: Usingtestutil.GenerateTestingAccountsWithPower
enhances flexibility in account generation for tests.x/trigger/simulation/genesis.go (4)
16-16
: Standardization of random number generation usinghelpers.RandIntBetween
improves maintainability and consistency.Also applies to: 34-34, 39-39, 47-47, 50-50, 56-56, 66-66
47-50
: Use ofhelpers.RandIntBetween
inNewRandomEvent
ensures controlled random value generation.
56-56
: Usinghelpers.RandIntBetween
inNewRandomAction
adds predictability to the simulation by ensuring amounts are within a specified range.
66-66
: Use ofhelpers.SelectRandomAccounts
inNewRandomTrigger
standardizes account selection, enhancing the robustness of simulations.x/name/simulation/operations.go (1)
17-17
: Incorporation ofhelpers.RandIntBetween
inSimulateMsgBindName
adds precision to the simulation by ensuring name lengths are within specified parameters.Also applies to: 70-70
x/name/simulation/operations_test.go (1)
211-211
: Usingtestutil.GenerateTestingAccountsWithPower
enhances flexibility in account generation for tests.x/oracle/client/cli/cli_test.go (3)
112-113
: LGTM! The use oftestutil
functions inGenerateAccountsWithKeyrings
ensures correct setup of accounts.Verification successful
The verification script successfully identified the usage of the
GenerateAccountsWithKeyrings
method in thex/oracle/client/cli/cli_test.go
file. This confirms that the method is being utilized correctly within the test suite.
s.GenerateAccountsWithKeyrings(2)
func (s *IntegrationTestSuite) GenerateAccountsWithKeyrings(number int)
The use of
testutil
functions inGenerateAccountsWithKeyrings
ensures the correct setup of accounts.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct setup of accounts using `testutil` functions. # Test: Check the setup of accounts in the test suite. Expect: Correctly initialized accounts. rg --type python $'GenerateAccountsWithKeyrings' x/oracle/client/cli/cli_test.goLength of output: 110
Script:
#!/bin/bash # Description: Verify the correct setup of accounts using `testutil` functions. # Test: Check the setup of accounts in the test suite. Expect: Correctly initialized accounts. rg --type go 'GenerateAccountsWithKeyrings' x/oracle/client/cli/cli_test.goLength of output: 184
101-101
: LGTM! The integration of CLI testing utilities inTestQueryOracleAddress
andTestOracleUpdate
enhances command handling.Also applies to: 112-113
Verification successful
The previous script failed due to an incorrect file type specification. The correct file type should be
go
instead ofpython
. Let's correct this and re-run the script to verify the usage of CLI testing utilities in the test methods.
The verification script has confirmed the usage of CLI testing utilities (
ExecTestCLICmd
andNewTxExecutor
) in the test methods. This indicates that the CLI commands are being handled correctly in the tests.
ExecTestCLICmd
is used in theTestQueryOracleAddress
method.NewTxExecutor
is used in theTestOracleUpdate
method.These findings align with the review comment, confirming that the integration of CLI testing utilities enhances command handling.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct handling of CLI commands in the test methods. # Test: Search for the usage of CLI testing utilities in the test methods. Expect: Effective use of utilities. rg --type python $'ExecTestCLICmd|NewTxExecutor' x/oracle/client/cli/cli_test.goLength of output: 112
Script:
#!/bin/bash # Description: Verify the correct handling of CLI commands in the test methods. # Test: Search for the usage of CLI testing utilities in the test methods. Expect: Effective use of utilities. rg --type go 'ExecTestCLICmd|NewTxExecutor' x/oracle/client/cli/cli_test.goLength of output: 297
35-43
: LGTM! The integration oftestutil
for keyring management enhances the setup process.Verification successful
The integration of
testutil
for keyring management and account addresses is consistent across thex/oracle/client/cli/cli_test.go
file.
keyring
andaccountAddresses
are used in various parts of the test suite, indicating proper integration.- The
keyring
is set up withtestutil.GenerateTestKeyring
.accountAddresses
are derived fromtestutil.GetKeyringEntryAddresses
.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of `testutil` keyring management with the rest of the test suite. # Test: Search for the usage of `keyring` and `accountAddresses` in the test suite. Expect: Consistent usage across the file. rg --type python $'keyring|accountAddresses' x/oracle/client/cli/cli_test.goLength of output: 108
Script:
#!/bin/bash # Description: Verify the integration of `testutil` keyring management with the rest of the test suite. # Test: Search for the usage of `keyring` and `accountAddresses` in the test suite. Expect: Consistent usage across the file. rg --type go 'keyring|accountAddresses' x/oracle/client/cli/cli_test.goLength of output: 1037
internal/helpers/rand_test.go (2)
15-95
: TheTestRandIntBetween
function is well-implemented with comprehensive test cases covering both normal and edge cases.
97-283
: TheTestSelectRandomEntries
function is effectively structured, providing a robust set of test cases that validate functionality across a variety of scenarios.x/sanction/client/testutil/cli_test.go (1)
32-32
: TheTestSanctionValidatorImmediateUsingGovCmds
function is well-structured and comprehensive, testing multiple aspects of the sanctioning process.x/msgfees/client/cli/cli_test.go (3)
36-36
: TheTestMsgFeesProposal
function is well-implemented, covering a comprehensive range of scenarios for message fee operations.
36-36
: TheTestUpdateNhashPerUsdMilProposal
function effectively tests the update functionality for the nhash per USD million rate, with appropriate error handling.
36-36
: TheTestUpdateConversionFeeDenomProposal
function is well-structured, effectively testing the update functionality for the conversion fee denomination with proper input validation.x/attribute/simulation/operations_test.go (1)
270-270
: RefactoredgetTestingAccounts
to usetestutil.GenerateTestingAccountsWithPower
. Ensure this integration is thoroughly tested.Verification successful
The search results indicate that the
getTestingAccounts
function has been updated to usetestutil.GenerateTestingAccountsWithPower
in multiple test files. This confirms that the integration of the new utility function is consistent across the codebase.
- The function
getTestingAccounts
inx/attribute/simulation/operations_test.go
and other test files now callstestutil.GenerateTestingAccountsWithPower
.- The integration appears to be correctly handled, as the function is used in various test scenarios without issues.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify integration of the new utility function in simulation tests. # Test: Search for the function usage in test files. Expect: Proper usage and handling of the function's return values. rg --type py "getTestingAccounts" --files-with-matches | xargs catLength of output: 66
Script:
#!/bin/bash # Description: Verify integration of the new utility function in simulation tests. # Test: Search for the function usage in Go test files. Expect: Proper usage and handling of the function's return values. rg --type go "getTestingAccounts" --files-with-matches | xargs catLength of output: 59066
testutil/sims.go (1)
22-361
: Introduced new utility functions and types for simulation testing. Ensure these are integrated and used correctly across the test suite.Verification successful
The new utility functions and types introduced in
testutil/sims.go
are indeed being used across various test files in the codebase. This confirms their integration and proper usage in the simulation tests.
- The
SimTestHelper
class and its methods are utilized in multiple test files to facilitate simulation testing.- Functions like
GenerateTestingAccounts
,AssertWeightedOperations
, and others are also being employed in the test suite.This comprehensive usage indicates that the new utilities are well-integrated and are serving their intended purpose effectively.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of new utility functions and types in simulation tests. # Test: Search for the usage of new functions and types. Expect: Proper usage and handling across the test suite. rg --type py "SimTestHelper|GenerateTestingAccounts|AssertWeightedOperations" --files-with-matches | xargs catLength of output: 112
Script:
#!/bin/bash # Description: Verify the usage of new utility functions and types in Go simulation tests. # Test: Search for the usage of new functions and types. Expect: Proper usage and handling across the test suite. rg --type go "SimTestHelper|GenerateTestingAccounts|AssertWeightedOperations" --files-with-matches | xargs catLength of output: 136134
internal/provwasm/simulation.go (4)
36-37
: Interface implementation forWrapper
is correctly declared.This ensures that
Wrapper
conforms to the expected interfaces for simulation modules in Cosmos SDK, which is crucial for integration with other parts of the system.
92-93
: Updated method name toProposalMsgs
to align with interface changes.This change is consistent with the updates in the Cosmos SDK, ensuring that the method correctly represents its functionality as returning weighted proposal messages for simulations.
Line range hint
58-71
: Proper setup of GenesisState inGenerateGenesisState
.The method correctly configures the genesis parameters for the wasm module, ensuring that the simulation environment is initialized with the appropriate settings.
Line range hint
120-128
: Well-structured method for weighted operations.The method effectively organizes simulation operations with their respective weights, facilitating a balanced simulation environment.
x/sanction/simulation/operations.go (2)
98-116
: The implementation ofProposalMsgs
function looks correct and well-structured.
545-547
: TheSimulatePropMsgUpdateParams
function is correctly implemented for simulating parameter update messages.x/marker/simulation/operations_test.go (3)
223-229
: TheresetParams
function correctly resets governance parameters to their default values for testing purposes.
461-461
: ThegetTestingAccounts
function is implemented correctly to generate test accounts using thetestutil
package.
497-497
: ThecreateTestingAccountsWithPower
function correctly creates testing accounts with specified power using thetestutil
package.x/trigger/client/cli/cli_test.go (3)
183-184
: Good use of utility functions to simplify keyring and address setup in tests.This approach enhances readability and maintainability of the test setup.
38-39
: Ensure proper cleanup of network resources in tests to prevent potential resource leaks.
172-172
: Consider handling potential errors fromWithKeyring
method to ensure robustness.x/exchange/client/cli/cli_test.go (3)
57-59
: Refactored keyring and account address handling to use new test utilities.This change aligns with the PR's objective to enhance testing utilities and refactor tests for efficiency. The use of
testutil.TestKeyringEntry
and the centralized generation of keyring entries and addresses improve maintainability and consistency across tests.
365-365
: Ensure the client context is correctly updated with the keyring.This line ensures that the validator's client context within the test network is updated to include the keyring, which is crucial for executing transactions that require authentication.
379-380
: Introduced utility functions for generating and retrieving keyring entries.The use of
testutil.GenerateTestKeyring
andtestutil.GetKeyringEntryAddresses
centralizes and simplifies the creation and management of test accounts, which is beneficial for maintaining the test suite.x/attribute/client/cli/cli_test.go (2)
41-45
: Refactor to use new test utilities for keyring management.The changes here streamline the process of generating keyrings and addresses by utilizing the new utility functions from the
testutil
package. This is a good use of shared utility functions to reduce code duplication and potential errors.
79-80
: Ensure correct integration ofGenerateTestKeyring
andGetKeyringEntryAddresses
.The integration of
GenerateTestKeyring
andGetKeyringEntryAddresses
from thetestutil
package is correctly implemented. This change simplifies the setup process for tests by encapsulating keyring creation and address extraction into utility functions, enhancing maintainability and readability.app/app.go (1)
773-773
: The removal ofapp.BankKeeper
fromibcratelimitmodule.NewAppModule
simplifies the dependencies of the IBC rate limit module. Ensure that this change aligns with the intended design and that theRateLimitingKeeper
no longer requires operations related to the bank module.Also applies to: 1018-1018
x/sanction/simulation/operations_test.go (6)
300-340
: Review the structure and assertions inTestProposalMsgs
.The test method
TestProposalMsgs
is well-structured and includes comprehensive checks for each aspect of the proposal messages. Good use of table-driven tests to cover different scenarios.
24-24
: Ensure new imports are utilized effectively.Verification successful
The new imports
testutil
andassertions
are effectively utilized in the filex/sanction/simulation/operations_test.go
.
testutil.GenerateTestingAccountsWithPower
is used.- Multiple functions from
assertions
are used, such asRequireNotPanicsNoError
andAssertErrorContents
.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the new imports are used in the file. rg --type go 'testutil|assertions' x/sanction/simulation/operations_test.goLength of output: 1002
1740-1759
: Ensure the random parameters generation is appropriate for the context.Verification successful
Ensure the random parameters generation is appropriate for the context.
The
RandomParams
function is defined inx/sanction/simulation/genesis.go
and generates random values forImmediateSanctionMinDeposit
andImmediateUnsanctionMinDeposit
based on a random number generator. This function appears to be suitable for generating random parameters for testing purposes.
x/sanction/simulation/genesis.go:67-76
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check that the random parameters generated are suitable for the tests and align with expected values. rg 'RandomParams' x/sanction/simulation/params.goLength of output: 121
Script:
#!/bin/bash # Search for the definition of the RandomParams function across the entire codebase ast-grep --lang go --pattern 'func RandomParams($_) $_'Length of output: 795
253-256
: Ensure weights are defined and correct.Verification successful
The weights
DefaultWeightSanction
,DefaultWeightSanctionImmediate
,DefaultWeightUnsanction
, andDefaultWeightUnsanctionImmediate
are defined inx/sanction/simulation/operations.go
and are all set to 10. This confirms that the weights used in the test are defined and correctly set.
DefaultWeightSanction = 10
DefaultWeightSanctionImmediate = 10
DefaultWeightUnsanction = 10
DefaultWeightUnsanctionImmediate = 10
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the weights used in the test are defined and correctly set in the configuration. rg 'DefaultWeightSanction|DefaultWeightSanctionImmediate|DefaultWeightUnsanction|DefaultWeightUnsanctionImmediate' x/sanction/simulation/params.goLength of output: 220
Script:
#!/bin/bash # Locate the file where the weights might be defined by searching the entire repository. rg 'DefaultWeightSanction|DefaultWeightSanctionImmediate|DefaultWeightUnsanction|DefaultWeightUnsanctionImmediate'Length of output: 1278
Line range hint
450-467
: Check error handling inSendGovMsg
.Verification successful
The error handling in the
SendGovMsg
function is robust and covers necessary cases. The function includes checks for errors when wrapping the message asAny
and when generating and delivering the transaction, returning appropriate error messages and the error itself.
x/sanction/simulation/operations.go
:
- Line 136: Checks for errors when wrapping the message as
Any
.- Line 162: Checks for errors when generating and delivering the transaction.
x/marker/simulation/operations.go
:
- Line 585: Checks for errors when wrapping the message as
Any
.- Line 612: Checks for errors when generating and delivering the transaction.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the error handling in SendGovMsg is robust and covers all necessary cases. rg 'SendGovMsg' x/sanction/simulation/helpers.goLength of output: 121
Script:
#!/bin/bash # Locate the file containing the definition of the SendGovMsg function. ast-grep --lang go --pattern 'func SendGovMsg($_) $_' # Once located, verify the error handling in the SendGovMsg function. ast-grep --lang go --pattern 'func SendGovMsg($_) $_ { $$$ if err != nil { $$$ } $$$ }'Length of output: 7024
836-836
: Validate the logic inMaxCoins
function.
… there aren't any accounts to select from.
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.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- x/oracle/simulation/genesis.go (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- x/oracle/simulation/genesis.go
Description
This PR does the following:
ProposalMsgs
module interface for the internal/provwasm, ibcratelimit, oracle, and sanction modules.This PR should not be merged unless all the unit tests pass (even the ones not currently required).
Once this PR has merged, I'll update the required actions to include all tests. Then I'll make a PR to remove the build-tests stuff action.
Related to:
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/
) or specification (x/<module>/spec/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorerCodecov Report
in the comment section below once CI passesSummary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests