refactor(tests): testutil.Network as an interface #18389
Conversation
WalkthroughWalkthroughThe changes involve a significant refactoring of the Changes
Assessment against linked issues (Beta)
TipsChat with CodeRabbit Bot (
|
github-prbot
requested review from
a team,
likhita-809 and
testinginprod
and removed request for
a team
![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: 25
Configuration used: CodeRabbit UI
Commits
Files that changed from the base of the PR and between 1e4d1f9 and 72080dbc5fc206a6d6d349e79c4a3dc3234f8331.Files ignored due to filter (1)
- x/auth/go.mod
Files selected for processing (28)
- client/grpc/cmtservice/status_test.go (1 hunks)
- client/rpc/rpc_test.go (3 hunks)
- server/api/server_test.go (5 hunks)
- simapp/testutil_network_test.go (1 hunks)
- tests/e2e/auth/suite.go (58 hunks)
- tests/e2e/authz/grpc.go (16 hunks)
- tests/e2e/authz/tx.go (35 hunks)
- tests/e2e/bank/grpc.go (8 hunks)
- tests/e2e/bank/suite.go (18 hunks)
- tests/e2e/client/grpc/cmtservice/service_test.go (10 hunks)
- tests/e2e/distribution/grpc_query_suite.go (17 hunks)
- tests/e2e/distribution/withdraw_all_suite.go (4 hunks)
- tests/e2e/gov/deposits.go (9 hunks)
- tests/e2e/gov/grpc.go (25 hunks)
- tests/e2e/gov/tx.go (19 hunks)
- tests/e2e/group/suite.go (8 hunks)
- tests/e2e/mint/grpc.go (2 hunks)
- tests/e2e/mint/suite.go (1 hunks)
- tests/e2e/staking/suite.go (4 hunks)
- tests/e2e/tx/benchmarks_test.go (7 hunks)
- tests/e2e/tx/service_test.go (42 hunks)
- tests/integration/server/grpc/server_test.go (8 hunks)
- testutil/cli/tx.go (2 hunks)
- testutil/network/interface.go (1 hunks)
- testutil/network/network.go (7 hunks)
- testutil/network/util.go (1 hunks)
- testutil/network/validator.go (1 hunks)
- x/auth/types/account_retriever_test.go (1 hunks)
Files not summarized due to errors (1)
- tests/e2e/auth/suite.go: Error: Message exceeds token limit
Additional comments: 167
simapp/testutil_network_test.go (1)
- 14-18: The type of
networkhas been changed from*network.Networktonetwork.NetworkI. Ensure that all methods called onnetworkare defined in theNetworkIinterface and that the actual object assigned tonetworkimplements this interface.testutil/network/validator.go (1)
- 1-85: The changes in this file are consistent with the goal of refactoring
testutil.Networkto an interface. TheValidatorstruct now implements theValidatorIinterface, and all fields are accessed through getter methods, improving encapsulation. Ensure that all references to the fields ofValidatorin the codebase have been updated to use these getter methods.client/grpc/cmtservice/status_test.go (1)
- 21-32: The changes in this hunk reflect the refactoring of the
networkandval0objects to encapsulate access to their internal state. The direct access tonetwork.Validatorshas been replaced with a call tonetwork.GetValidators(). Similarly, the method calls onval0have been updated fromval0.ClientCtxtoval0.GetClientCtx()andval0.Monikertoval0.GetMoniker(). These changes improve the encapsulation and maintainability of the codebase.testutil/network/util.go (1)
- 44-48: The change from passing a dereferenced
valto passing a pointer tovalin thecfg.AppConstructor(val)call is a significant one. It means that theAppConstructorfunction can now modify thevalobject. Ensure that this is the intended behavior and that it doesn't introduce any unintended side effects.testutil/network/interface.go (1)
- 1-43: The introduction of the
NetworkIandValidatorIinterfaces is a good practice as it allows for better abstraction and flexibility. It also improves the maintainability of the codebase by allowing different implementations of these interfaces to be swapped in as needed. However, ensure that all the methods defined in these interfaces are implemented wherever these interfaces are used.tests/e2e/mint/grpc.go (2)
16-18: The changes here are consistent with the pull request summary. The direct field access has been replaced with getter methods, which is a good practice for encapsulation.
57-63: The changes here are also consistent with the pull request summary. The
GetClientCtx().Codec.UnmarshalJSONmethod is used instead of direct field access, which is a good practice for encapsulation and access control.testutil/cli/tx.go (2)
16-19: The function signature has been updated to use the
NetworkIinterface instead of a concreteNetworktype. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.44-47: The function signature has been updated to use the
NetworkIinterface instead of a concreteNetworktype. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.tests/e2e/mint/suite.go (2)
13-17: The
networkfield in theE2ETestSuitestruct has been updated from a pointer to an interface typenetwork.NetworkI. This change is in line with the refactor to makenetwork.Networkan interface. Ensure that all usages of this field are updated to use the methods defined in thenetwork.NetworkIinterface.19-19: The
NewE2ETestSuitefunction signature remains unchanged, but the function body may need to be updated to correctly initialize thenetworkfield with an instance of a type that implements thenetwork.NetworkIinterface.client/rpc/rpc_test.go (3)
22-26: The
networkfield has been changed from a pointer to an interface typenetwork.NetworkI. This is a good practice as it allows for easy swapping and use of different implementations.50-52: Direct field access
s.network.Validators[0].ClientCtxhas been replaced with getter methodss.network.GetValidators()[0].GetClientCtx(). This is a good practice as it improves encapsulation.91-105: Direct field access
s.network.Validators[0]has been replaced with getter methodss.network.GetValidators()[0]. This is a good practice as it improves encapsulation.x/auth/types/account_retriever_test.go (4)
23-24: Ensure that the
WaitForHeightfunction handles the newzargument correctly.26-27: The direct access to fields has been replaced with getter methods, which is a good practice for encapsulation.
30-30: The
WithHeightmethod is used to set the height forclientCtx. Ensure that this method is implemented correctly and that it doesn't introduce any side effects.32-46: The function signatures for
GetAccount,GetAccountWithHeight,EnsureExists, andGetAccountNumberSequenceremain the same, but the way the arguments are passed has been updated to use getter methods. Ensure that these changes do not affect the functionality of these methods.tests/e2e/tx/benchmarks_test.go (7)
24-27: The
networkfield in theE2EBenchmarkSuitestruct has been changed from a pointer to an interface typenetwork.NetworkI. This change is in line with the goal of the pull request to refactor thetestutil.Networkto an interface.44-55: The
networkfield is now accessed through getter methods instead of direct field access. This is a good practice as it improves encapsulation.95-116: The
networkfield is now accessed through getter methods instead of direct field access. This is a good practice as it improves encapsulation.119-137: The
networkfield is now accessed through getter methods instead of direct field access. This is a good practice as it improves encapsulation.142-151: The
networkfield is now accessed through getter methods instead of direct field access. This is a good practice as it improves encapsulation.158-174: The
networkfield is now accessed through getter methods instead of direct field access. This is a good practice as it improves encapsulation.178-190: The
networkfield is now accessed through getter methods instead of direct field access. This is a good practice as it improves encapsulation.tests/e2e/staking/suite.go (4)
24-28: The
networkfield in theE2ETestSuitestruct has been changed from a pointer to an interface typenetwork.NetworkI. This change allows for easy swapping and use of different implementations of the network. Ensure that all usages of this field throughout the codebase have been updated to match the new type.56-56: Direct field access
s.network.Validators[0]has been replaced with a getter methods.network.GetValidators()[0]. This change improves encapsulation and makes the code more maintainable.98-98: Direct field access
val.RPCAddresshas been replaced with a getter methodval.GetRPCAddress(). This change improves encapsulation and makes the code more maintainable.117-118: Direct field access
val.PubKey.Bytes()has been replaced with a getter methodval.GetPubKey().Bytes(). This change improves encapsulation and makes the code more maintainable.tests/e2e/client/grpc/cmtservice/service_test.go (3)
26-31: The
networkfield in theE2ETestSuitestruct has been updated to use thenetwork.NetworkIinterface instead of thenetwork.Networktype. This change allows for greater flexibility in the network implementation used in the test suite.47-51: The
GetValidatorsmethod is used instead of directly accessing theValidatorsfield. This change is part of the move towards using getter methods instead of direct field access, improving encapsulation.234-250: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [205-261]
The
GetValidators,GetAPIAddress,GetClientCtx, andGetPubKeymethods are used instead of directly accessing the corresponding fields. This change is part of the move towards using getter methods instead of direct field access, improving encapsulation.tests/e2e/distribution/withdraw_all_suite.go (4)
24-28: The
networkfield in theWithdrawAllTestSuitestruct has been updated to use thenetwork.NetworkIinterface type instead of a concrete type. This change improves the flexibility and testability of the code by allowing different implementations of theNetworkIinterface to be used.52-60: The
networkfield is now accessed through getter methods likeGetValidators()andGetClientCtx(). This is a good practice as it encapsulates the internal structure of thenetworkobject, improving maintainability.63-76: The
networkfield is accessed through getter methods likeGetAddress()andGetClientCtx(). This is a good practice as it encapsulates the internal structure of thenetworkobject, improving maintainability.80-101: The
networkfield is accessed through getter methods likeGetClientCtx(),GetValAddress(). This is a good practice as it encapsulates the internal structure of thenetworkobject, improving maintainability.tests/e2e/bank/suite.go (3)
23-27: The
networkfield in theE2ETestSuitestruct has been changed from a pointer to an interface typenetwork.NetworkI. This change is part of the refactor to makenetworkan interface, which will improve the flexibility and maintainability of the codebase.336-341: The
network.Validatorsfield has been replaced with a call tonetwork.GetValidators(). This change is part of the refactor to makenetworkan interface, which will improve the encapsulation of the codebase.361-361: The
networkfield in various structs has been changed from a pointer to an interface typenetwork.NetworkI. This change is part of the refactor to makenetworkan interface, which will improve the flexibility and maintainability of the codebase.tests/e2e/gov/deposits.go (4)
22-26: The
networkfield in theDepositTestSuitestruct has been changed from a pointer to an interface typenetwork.NetworkI. This change improves the flexibility and maintainability of the codebase by allowing different implementations of theNetworkIinterface to be used.40-52: The
submitProposalmethod now takes anetwork.ValidatorIinterface instead of a*network.Validator. This change improves the flexibility and maintainability of the codebase by allowing different implementations of theValidatorIinterface to be used. TheGetClientCtxandGetAddressmethods have been introduced to retrieve client context and address from thenetwork.ValidatorIinterface.148-154: The
network.GetValidators()method is used to retrieve the validators from the network. This change improves the encapsulation of the codebase by replacing direct field access with getter methods.160-160: The
GetClientCtxmethod is used to retrieve the client context from thenetwork.ValidatorIinterface. This change improves the encapsulation of the codebase by replacing direct field access with getter methods.server/api/server_test.go (5)
40-45: The
networkfield in theGRPCWebTestSuitestruct has been changed from a pointer to an interface typenetwork.NetworkI. This change allows for easy swapping and use of different implementations, improving flexibility and testability.71-74: The
s.network.Validatorshas been replaced with a call tos.network.GetValidators(). This change improves encapsulation by replacing direct field access with getter methods.84-87: The
val.PubKeyhas been replaced with a call toval.GetPubKey(). This change improves encapsulation by replacing direct field access with getter methods.131-134: The
s.network.Validatorshas been replaced with a call tos.network.GetValidators(). This change improves encapsulation by replacing direct field access with getter methods.149-152: The
val.AppConfig.API.Addresshas been replaced with a call toval.GetAppConfig().API.Address. This change improves encapsulation by replacing direct field access with getter methods.tests/e2e/distribution/grpc_query_suite.go (1)
- 20-24: The
networkfield in theGRPCQueryTestSuitestruct has been changed from a pointer to an interface typenetwork.NetworkI. This change is part of the refactor to makenetworkan interface, which will allow for easier swapping and use of different implementations.tests/integration/server/grpc/server_test.go (8)
33-38: The
networkfield in theIntegrationTestSuitestruct has been changed from a pointer to an interface typenetwork.NetworkI. This change is in line with the goal of improving encapsulation and code maintainability.54-59: Direct field access has been replaced with getter methods. This is a good practice as it improves encapsulation and code maintainability.
78-94: Direct field access has been replaced with getter methods. This is a good practice as it improves encapsulation and code maintainability.
99-103: Direct field access has been replaced with getter methods. This is a good practice as it improves encapsulation and code maintainability.
167-174: Direct field access has been replaced with getter methods. This is a good practice as it improves encapsulation and code maintainability.
220-226: Direct field access has been replaced with getter methods. This is a good practice as it improves encapsulation and code maintainability.
240-254: Direct field access has been replaced with getter methods. This is a good practice as it improves encapsulation and code maintainability.
258-270: Direct field access has been replaced with getter methods. This is a good practice as it improves encapsulation and code maintainability.
tests/e2e/bank/grpc.go (8)
18-20: The direct access to the
Validatorsfield has been replaced with a call toGetValidators(). This change is consistent with the encapsulation principle. Ensure that theGetValidators()method is implemented correctly and returns the expected data.37-38: The direct access to the
Monikerfield has been replaced with a call toGetMoniker(). This change is consistent with the encapsulation principle. Ensure that theGetMoniker()method is implemented correctly and returns the expected data.104-106: The direct access to the
Validatorsfield has been replaced with a call toGetValidators(). This change is consistent with the encapsulation principle. Ensure that theGetValidators()method is implemented correctly and returns the expected data.229-231: The direct access to the
Validatorsfield has been replaced with a call toGetValidators(). This change is consistent with the encapsulation principle. Ensure that theGetValidators()method is implemented correctly and returns the expected data.240-240: The direct access to the
Addressfield has been replaced with a call toGetAddress(). This change is consistent with the encapsulation principle. Ensure that theGetAddress()method is implemented correctly and returns the expected data.244-245: The direct access to the
Monikerfield has been replaced with a call toGetMoniker(). This change is consistent with the encapsulation principle. Ensure that theGetMoniker()method is implemented correctly and returns the expected data.254-254: The direct access to the
Addressfield has been replaced with a call toGetAddress(). This change is consistent with the encapsulation principle. Ensure that theGetAddress()method is implemented correctly and returns the expected data.265-265: The direct access to the
Addressfield has been replaced with a call toGetAddress(). This change is consistent with the encapsulation principle. Ensure that theGetAddress()method is implemented correctly and returns the expected data.tests/e2e/auth/suite.go (3)
120-121: The home directory is being replaced from "simd" to "simcli". Ensure that this change is intentional and that the new directory exists and has the correct permissions.
226-227: The home directory is being replaced from "simd" to "simcli". Ensure that this change is intentional and that the new directory exists and has the correct permissions.
797-798: The home directory is being replaced from "simd" to "simcli". Ensure that this change is intentional and that the new directory exists and has the correct permissions.
tests/e2e/authz/grpc.go (17)
19-22: The direct field access
s.network.Validators[0]has been replaced with the getter methods.network.GetValidators()[0]. This change improves encapsulation and maintainability.33-38: The direct field access
val.Address.String()has been replaced with the getter methodval.GetAddress().String(). This change improves encapsulation and maintainability.71-80: The direct field access
val.ClientCtxhas been replaced with the getter methodval.GetClientCtx(). This change improves encapsulation and maintainability.88-93: The direct field access
s.network.Validators[0]has been replaced with the getter methods.network.GetValidators()[0]. This change improves encapsulation and maintainability.101-104: The direct field access
val.Address.String()has been replaced with the getter methodval.GetAddress().String(). This change improves encapsulation and maintainability.115-121: The direct field access
val.ClientCtxhas been replaced with the getter methodval.GetClientCtx(). This change improves encapsulation and maintainability.134-137: The direct field access
val.Address.String()has been replaced with the getter methodval.GetAddress().String(). This change improves encapsulation and maintainability.144-147: The direct field access
val.Address.String()has been replaced with the getter methodval.GetAddress().String(). This change improves encapsulation and maintainability.161-167: The direct field access
val.ClientCtxhas been replaced with the getter methodval.GetClientCtx(). This change improves encapsulation and maintainability.173-176: The direct field access
s.network.Validators[0]has been replaced with the getter methods.network.GetValidators()[0]. This change improves encapsulation and maintainability.186-190: The direct field access
val.Address.String()has been replaced with the getter methodval.GetAddress().String(). This change improves encapsulation and maintainability.200-203: The direct field access
val.Address.String()has been replaced with the getter methodval.GetAddress().String(). This change improves encapsulation and maintainability.212-218: The direct field access
val.ClientCtxhas been replaced with the getter methodval.GetClientCtx(). This change improves encapsulation and maintainability.224-227: The direct field access
s.network.Validators[0]has been replaced with the getter methods.network.GetValidators()[0]. This change improves encapsulation and maintainability.237-241: The direct field access
val.Address.String()has been replaced with the getter methodval.GetAddress().String(). This change improves encapsulation and maintainability.251-254: The direct field access
val.Address.String()has been replaced with the getter methodval.GetAddress().String(). This change improves encapsulation and maintainability.263-269: The direct field access
val.ClientCtxhas been replaced with the getter methodval.GetClientCtx(). This change improves encapsulation and maintainability.tests/e2e/group/suite.go (8)
27-31: The
networkfield in theE2ETestSuitestruct has been changed from a pointer to an interface typenetwork.NetworkI. This change allows for easy swapping and use of different implementations of the network.57-82: The
networkfield is now accessed through getter methods instead of direct field access. This improves encapsulation and maintainability of the codebase. Also,val.GetValidators()is used instead ofval.Validatorsandval.GetClientCtx()is used instead ofval.ClientCtx.96-107: The
networkfield is now accessed through getter methods instead of direct field access. This improves encapsulation and maintainability of the codebase. Also,val.GetValidators()is used instead ofval.Validatorsandval.GetClientCtx()is used instead ofval.ClientCtx.122-141: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [110-133]
The
networkfield is now accessed through getter methods instead of direct field access. This improves encapsulation and maintainability of the codebase. Also,val.GetValidators()is used instead ofval.Validatorsandval.GetClientCtx()is used instead ofval.ClientCtx.
134-141: The
networkfield is now accessed through getter methods instead of direct field access. This improves encapsulation and maintainability of the codebase. Also,val.GetValidators()is used instead ofval.Validatorsandval.GetClientCtx()is used instead ofval.ClientCtx.144-204: The
networkfield is now accessed through getter methods instead of direct field access. This improves encapsulation and maintainability of the codebase. Also,val.GetValidators()is used instead ofval.Validatorsandval.GetClientCtx()is used instead ofval.ClientCtx.241-248: The
networkfield is now accessed through getter methods instead of direct field access. This improves encapsulation and maintainability of the codebase. Also,val.GetValidators()is used instead ofval.Validatorsandval.GetClientCtx()is used instead ofval.ClientCtx.257-285: The
networkfield is now accessed through getter methods instead of direct field access. This improves encapsulation and maintainability of the codebase. Also,val.GetValidators()is used instead ofval.Validatorsandval.GetClientCtx()is used instead ofval.ClientCtx.testutil/network/network.go (4)
268-269: Ensure that all references to the
Configfield have been updated to use the getter method.280-281: Ensure that the
Loggerinterface is implemented correctly by*testing.Tand*CLILogger.302-308: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [305-309]
Ensure that the
Newfunction is updated everywhere it's called.
- 685-691: The
GetValidatorsmethod is a good addition for encapsulation. Ensure that all direct accesses to theValidatorsfield have been replaced with this method.tests/e2e/gov/tx.go (16)
26-30: The
networkfield in theE2ETestSuitestruct has been changed from a pointer to an interface typenetwork.NetworkI. This change improves the flexibility and maintainability of the codebase by allowing different implementations of theNetworkIinterface to be used interchangeably.41-67: The
networkfield is now accessed through getter methods instead of direct field access. This change improves encapsulation and makes the code more robust to future changes in theNetworkIinterface.98-103: The
networkfield is now accessed through getter methods instead of direct field access. This change improves encapsulation and makes the code more robust to future changes in theNetworkIinterface.154-160: The
networkfield is now accessed through getter methods instead of direct field access. This change improves encapsulation and makes the code more robust to future changes in theNetworkIinterface.168-174: The
networkfield is now accessed through getter methods instead of direct field access. This change improves encapsulation and makes the code more robust to future changes in theNetworkIinterface.186-190: The
networkfield is now accessed through getter methods instead of direct field access. This change improves encapsulation and makes the code more robust to future changes in theNetworkIinterface.213-219: The
networkfield is now accessed through getter methods instead of direct field access. This change improves encapsulation and makes the code more robust to future changes in theNetworkIinterface.225-231: The
networkfield is now accessed through getter methods instead of direct field access. This change improves encapsulation and makes the code more robust to future changes in theNetworkIinterface.236-242: The
networkfield is now accessed through getter methods instead of direct field access. This change improves encapsulation and makes the code more robust to future changes in theNetworkIinterface.250-256: The
networkfield is now accessed through getter methods instead of direct field access. This change improves encapsulation and makes the code more robust to future changes in theNetworkIinterface.264-270: The
networkfield is now accessed through getter methods instead of direct field access. This change improves encapsulation and makes the code more robust to future changes in theNetworkIinterface.282-286: The
networkfield is now accessed through getter methods instead of direct field access. This change improves encapsulation and makes the code more robust to future changes in theNetworkIinterface.298-304: The
networkfield is now accessed through getter methods instead of direct field access. This change improves encapsulation and makes the code more robust to future changes in theNetworkIinterface.335-341: The
networkfield is now accessed through getter methods instead of direct field access. This change improves encapsulation and makes the code more robust to future changes in theNetworkIinterface.347-353: The
networkfield is now accessed through getter methods instead of direct field access. This change improves encapsulation and makes the code more robust to future changes in theNetworkIinterface.360-366: The
networkfield is now accessed through getter methods instead of direct field access. This change improves encapsulation and makes the code more robust to future changes in theNetworkIinterface.tests/e2e/authz/tx.go (23)
37-37: The
networkfield in theE2ETestSuitestruct has been updated to use thenetwork.NetworkItype. Ensure that all usages of this field are updated accordingly.52-52: The
valvariable is now assigned usings.network.GetValidators()[0]instead ofval.ClientCtxandval.Address. Ensure that this change is reflected in all usages ofval.61-61: The function call to
govtestutil.MsgSubmitLegacyProposalhas been updated to useval.GetClientCtx()andval.GetAddress().String(). Ensure that this change is reflected in all usages of this function.73-77: The function call to
authzclitestutil.CreateGranthas been updated. Ensure that this change is reflected in all usages of this function.86-87: The function calls to
val.GetClientCtx().Codec.UnmarshalJSONandclitestutil.CheckTxCodehave been updated to useval.GetClientCtx(). Ensure that this change is reflected in all usages of these functions.114-120: The function call to
authzclitestutil.CreateGranthas been updated. Ensure that this change is reflected in all usages of this function.130-131: The function calls to
val.GetClientCtx().Codec.UnmarshalJSONandclitestutil.CheckTxCodehave been updated to useval.GetClientCtx(). Ensure that this change is reflected in all usages of these functions.134-137: The function call to
val.GetClientCtx().Keyring.NewMnemonichas been updated to useval.GetClientCtx(). Ensure that this change is reflected in all usages of this function.147-150: The function call to
val.GetAddress()has been updated. Ensure that this change is reflected in all usages of this function.159-159: The function call to
clitestutil.SubmitTestTxhas been updated to useval.GetClientCtx(). Ensure that this change is reflected in all usages of this function.181-195: The function call to
authzclitestutil.CreateGranthas been updated. Ensure that this change is reflected in all usages of this function.224-234: The function call to
authzclitestutil.CreateGranthas been updated. Ensure that this change is reflected in all usages of this function.311-311: The function call to
val.GetClientCtx()has been updated. Ensure that this change is reflected in all usages of this function.320-320: The function call to
clitestutil.CheckTxCodehas been updated to useval.GetClientCtx(). Ensure that this change is reflected in all usages of this function.327-338: The function call to
authzclitestutil.CreateGranthas been updated. Ensure that this change is reflected in all usages of this function.358-358: The function call to
clitestutil.SubmitTestTxhas been updated to useval.GetClientCtx(). Ensure that this change is reflected in all usages of this function.421-421: The function call to
val.GetClientCtx()has been updated. Ensure that this change is reflected in all usages of this function.436-436: The function call to
clitestutil.CheckTxCodehas been updated to useval.GetClientCtx(). Ensure that this change is reflected in all usages of this function.443-456: The function call to
authzclitestutil.CreateGranthas been updated. Ensure that this change is reflected in all usages of this function.477-477: The function call to
clitestutil.SubmitTestTxhas been updated to useval.GetClientCtx(). Ensure that this change is reflected in all usages of this function.494-494: The function call to
clitestutil.SubmitTestTxhas been updated to useval.GetClientCtx(). Ensure that this change is reflected in all usages of this function.515-515: The function call to
val.GetClientCtx()has been updated. Ensure that this change is reflected in all usages of this function.528-528: The function call to
val.GetClientCtx()has been updated. Ensure that this change is reflected in all usages of this function.tests/e2e/gov/grpc.go (21)
13-19: The direct field access
s.network.Validators[0]has been replaced with the getter methods.network.GetValidators()[0]. This change improves encapsulation and makes the code more maintainable.22-38: The direct field access
val.APIAddresshas been replaced with the getter methodval.GetAPIAddress(). This change improves encapsulation and makes the code more maintainable.44-50: The direct field access
val.ClientCtx.Codechas been replaced with the getter methodval.GetClientCtx().Codec. This change improves encapsulation and makes the code more maintainable.57-63: The direct field access
s.network.Validators[0]has been replaced with the getter methods.network.GetValidators()[0]. This change improves encapsulation and makes the code more maintainable.70-74: The direct field access
val.APIAddresshas been replaced with the getter methodval.GetAPIAddress(). This change improves encapsulation and makes the code more maintainable.98-104: The direct field access
val.ClientCtx.Codechas been replaced with the getter methodval.GetClientCtx().Codec. This change improves encapsulation and makes the code more maintainable.113-119: The direct field access
s.network.Validators[0]has been replaced with the getter methods.network.GetValidators()[0]. This change improves encapsulation and makes the code more maintainable.125-129: The direct field access
val.APIAddressandval.Addresshave been replaced with the getter methodsval.GetAPIAddress()andval.GetAddress(). This change improves encapsulation and makes the code more maintainable.165-171: The direct field access
val.ClientCtx.Codechas been replaced with the getter methodval.GetClientCtx().Codec. This change improves encapsulation and makes the code more maintainable.183-189: The direct field access
s.network.Validators[0]has been replaced with the getter methods.network.GetValidators()[0]. This change improves encapsulation and makes the code more maintainable.194-196: The direct field access
val.APIAddresshas been replaced with the getter methodval.GetAPIAddress(). This change improves encapsulation and makes the code more maintainable.209-215: The direct field access
val.ClientCtx.Codechas been replaced with the getter methodval.GetClientCtx().Codec. This change improves encapsulation and makes the code more maintainable.222-228: The direct field access
s.network.Validators[0]has been replaced with the getter methods.network.GetValidators()[0]. This change improves encapsulation and makes the code more maintainable.233-235: The direct field access
val.APIAddressandval.Addresshave been replaced with the getter methodsval.GetAPIAddress()andval.GetAddress(). This change improves encapsulation and makes the code more maintainable.258-264: The direct field access
val.ClientCtx.Codechas been replaced with the getter methodval.GetClientCtx().Codec. This change improves encapsulation and makes the code more maintainable.271-277: The direct field access
s.network.Validators[0]has been replaced with the getter methods.network.GetValidators()[0]. This change improves encapsulation and makes the code more maintainable.282-284: The direct field access
val.APIAddresshas been replaced with the getter methodval.GetAPIAddress(). This change improves encapsulation and makes the code more maintainable.297-303: The direct field access
val.ClientCtx.Codechas been replaced with the getter methodval.GetClientCtx().Codec. This change improves encapsulation and makes the code more maintainable.311-317: The direct field access
s.network.Validators[0]has been replaced with the getter methods.network.GetValidators()[0]. This change improves encapsulation and makes the code more maintainable.322-324: The direct field access
val.APIAddresshas been replaced with the getter methodval.GetAPIAddress(). This change improves encapsulation and makes the code more maintainable.342-348: The direct field access
val.ClientCtx.Codechas been replaced with the getter methodval.GetClientCtx().Codec. This change improves encapsulation and makes the code more maintainable.
[OVERVIEWtests/e2e/tx/service_test.go (10)
45-45: The
networkfield in theE2ETestSuitestruct has been replaced withnetwork.NetworkItype. This change is a good practice as it allows for better abstraction and easier testing by using an interface instead of a concrete type. Ensure that all usages of this field have been updated to use thenetwork.NetworkIinterface methods instead of direct field access.1053-1053: The
GetValidators()method is used instead of direct field access. This is a good practice as it encapsulates the internal structure of the object and provides a clear interface for accessing the data.1080-1080: The
GetClientCtx().Codec.UnmarshalJSON()method is used to unmarshal JSON data. This is a good practice as it provides a clear and consistent interface for unmarshalling JSON data.1005-1009: The
GetClientCtx()method is used to get the client context. This is a good practice as it encapsulates the internal structure of the object and provides a clear interface for accessing the client context.1080-1080: The
GetClientCtx().Codec.UnmarshalJSON()method is used to unmarshal JSON data. This is a good practice as it provides a clear and consistent interface for unmarshalling JSON data.1005-1009: The
GetClientCtx()method is used to get the client context. This is a good practice as it encapsulates the internal structure of the object and provides a clear interface for accessing the client context.1080-1080: The
GetClientCtx().Codec.UnmarshalJSON()method is used to unmarshal JSON data. This is a good practice as it provides a clear and consistent interface for unmarshalling JSON data.1005-1009: The
GetClientCtx()method is used to get the client context. This is a good practice as it encapsulates the internal structure of the object and provides a clear interface for accessing the client context.1080-1080: The
GetClientCtx().Codec.UnmarshalJSON()method is used to unmarshal JSON data. This is a good practice as it provides a clear and consistent interface for unmarshalling JSON data.1005-1009: The
GetClientCtx()
tac0turtle
force-pushed
the
marko/18145
branch
from
72080db to
4602c7f
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 56
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- x/auth/go.mod
- x/auth/go.sum
Files selected for processing (28)
- client/grpc/cmtservice/status_test.go (1 hunks)
- client/rpc/rpc_test.go (3 hunks)
- server/api/server_test.go (5 hunks)
- simapp/testutil_network_test.go (1 hunks)
- tests/e2e/auth/suite.go (58 hunks)
- tests/e2e/authz/grpc.go (16 hunks)
- tests/e2e/authz/tx.go (35 hunks)
- tests/e2e/bank/grpc.go (8 hunks)
- tests/e2e/bank/suite.go (18 hunks)
- tests/e2e/client/grpc/cmtservice/service_test.go (10 hunks)
- tests/e2e/distribution/grpc_query_suite.go (17 hunks)
- tests/e2e/distribution/withdraw_all_suite.go (4 hunks)
- tests/e2e/gov/deposits.go (9 hunks)
- tests/e2e/gov/grpc.go (25 hunks)
- tests/e2e/gov/tx.go (19 hunks)
- tests/e2e/group/suite.go (8 hunks)
- tests/e2e/mint/grpc.go (2 hunks)
- tests/e2e/mint/suite.go (1 hunks)
- tests/e2e/staking/suite.go (4 hunks)
- tests/e2e/tx/benchmarks_test.go (7 hunks)
- tests/e2e/tx/service_test.go (42 hunks)
- tests/integration/server/grpc/server_test.go (8 hunks)
- testutil/cli/tx.go (2 hunks)
- testutil/network/interface.go (1 hunks)
- testutil/network/network.go (7 hunks)
- testutil/network/util.go (1 hunks)
- testutil/network/validator.go (1 hunks)
- x/auth/types/account_retriever_test.go (1 hunks)
Files not summarized due to errors (1)
- tests/e2e/auth/suite.go: Error: Message exceeds token limit
Files skipped from review due to trivial changes (1)
- tests/e2e/gov/grpc.go
Additional comments: 125
tests/e2e/mint/grpc.go (2)
16-18: The changes here are in line with the encapsulation principle. Direct access to fields has been replaced with getter methods, which is a good practice.
57-63: The changes here are also in line with the encapsulation principle. The
GetClientCtxmethod is used instead of directly accessing the field. This is a good practice.simapp/testutil_network_test.go (1)
- 14-18: The type of the
networkfield in theIntegrationTestSuitestruct has been changed from*network.Networktonetwork.NetworkI. This change is likely due to the refactoring of thenetworkpackage to use interfaces. Ensure that all methods and functions that use thenetworkfield ofIntegrationTestSuitehave been updated to use the new interface methods instead of direct field access.testutil/network/validator.go (1)
- 1-84: The
Validatorstruct and its methods look good. The encapsulation of fields with getter methods is a good practice for maintainability and data safety. However, ensure that all references to the fields of theValidatorstruct throughout the codebase have been updated to use these new getter methods.testutil/cli/tx.go (2)
16-19: The function signature has been updated to use the
NetworkIinterface instead of a concrete*network.Networktype. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.44-47: The function signature has been updated to use the
NetworkIinterface instead of a concrete*network.Networktype. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.testutil/network/util.go (1)
- 44-48: The change from dereferencing
valto passing the pointer directly tocfg.AppConstructorcould potentially alter the behavior of the function if it modifies thevalobject. Ensure that this change is intended and that it doesn't introduce any unintended side effects.- app := cfg.AppConstructor(*val) + app := cfg.AppConstructor(val)tests/e2e/bank/grpc.go (7)
18-20: The direct access to the validator's properties has been replaced with getter methods, which is a good practice for encapsulation and code maintainability.
37-39: The same encapsulation principle has been applied here as well.
104-106: The encapsulation principle is consistently applied throughout the code.
240-246: The encapsulation principle is consistently applied throughout the code.
254-257: The encapsulation principle is consistently applied throughout the code.
265-268: The encapsulation principle is consistently applied throughout the code.
279-283: The encapsulation principle is consistently applied throughout the code.
testutil/network/interface.go (1)
- 1-44: The introduction of the
NetworkIandValidatorIinterfaces is a good practice as it allows for better abstraction and encapsulation. It also improves the testability of the code by allowing different implementations to be swapped in for testing. However, ensure that all methods defined in these interfaces are implemented in the concrete types that will be using them.client/grpc/cmtservice/status_test.go (1)
- 21-32: The changes in this hunk are consistent with the pull request's goal of refactoring the
testutil.Networkinto an interface. The direct access tonetwork.Validatorshas been replaced with a call tonetwork.GetValidators(). Similarly, the method callsval0.ClientCtxandval0.Monikerhave been updated toval0.GetClientCtx()andval0.GetMoniker(), respectively. This improves code maintainability and encapsulation.tests/e2e/mint/suite.go (1)
- 13-17: The
networkfield in theE2ETestSuitestruct has been updated from a pointer to an interface typenetwork.NetworkI. This change likely affects how theE2ETestSuiteinteracts with thenetworkpackage. Ensure that all methods and functions that interact with thenetworkfield ofE2ETestSuitehave been updated to use the new interface type.tests/e2e/bank/suite.go (1)
- 23-27: The
networkfield in theE2ETestSuitestruct has been changed from a pointer to an interface typenetwork.NetworkI. This change allows for better abstraction and flexibility in the test suite, as it can now work with any type that implements theNetworkIinterface.tests/e2e/distribution/grpc_query_suite.go (3)
20-24: The
networkfield in theGRPCQueryTestSuitestruct has been changed from a concrete type to an interface typeNetworkI. This change allows for better abstraction and flexibility, as any type that implements theNetworkIinterface can now be used as thenetworkfield.411-412: Direct field access has been replaced with getter methods, such as
s.network.GetValidators()[0]andval.GetAPIAddress(). This is a good practice as it improves encapsulation.449-457: The error handling in these lines is correct and follows best practices. The error returned by
sdktestutil.GetRequestorsdktestutil.GetRequestWithHeadersis checked before proceeding with the test case. If an error is expected (tc.expErris true), the test asserts that an error occurs when unmarshalling the response. If an error is not expected, the test asserts that no error occurred during the request or unmarshalling, and that the unmarshalled response matches the expected response.tests/e2e/staking/suite.go (1)
- 24-28: The
networkfield in theE2ETestSuitestruct has been changed from*network.Networktonetwork.NetworkI. This change reflects the shift towards using interfaces for network interactions.client/rpc/rpc_test.go (3)
22-26: The change from
*network.Networktonetwork.NetworkIin theIntegrationTestSuitestruct is a good practice as it allows for better abstraction and easier testing with different implementations of theNetworkIinterface. Ensure that all instances ofIntegrationTestSuitehave been updated to use the new interface type.47-53: The change from
s.network.Validators[0].ClientCtxtos.network.GetValidators()[0].GetClientCtx()is a good practice as it encapsulates the access to theClientCtxfield, improving code maintainability and encapsulation. Ensure that all instances ofs.network.Validators[0].ClientCtxhave been updated to use the new method.91-105: The changes from
s.network.Validators[0]tos.network.GetValidators()[0]andval.Addresstoval.GetAddress()are good practices as they encapsulate the access to theValidatorsandAddressfields, improving code maintainability and encapsulation. Ensure that all instances ofs.network.Validators[0]andval.Addresshave been updated to use the new methods.x/auth/types/account_retriever_test.go (7)
23-24: Ensure that the
WaitForHeightmethod is correctly implemented and that the height of 3 is reached before proceeding.26-27: The use of the
GetValidatorsandGetClientCtxmethods improves encapsulation and maintainability.30-30: Ensure that the
WithHeightmethod correctly modifies theclientCtxobject.32-33: Ensure that the
GetAccountmethod correctly retrieves the account associated with the address returned byval.GetAddress().36-39: Ensure that the
GetAccountWithHeightmethod correctly retrieves the account and height associated with the address returned byval.GetAddress().41-41: Ensure that the
EnsureExistsmethod correctly checks the existence of the account associated with the address returned byval.GetAddress().43-46: Ensure that the
GetAccountNumberSequencemethod correctly retrieves the account number and sequence associated with the address returned byval.GetAddress().tests/e2e/gov/deposits.go (6)
22-26: The
networkfield's type has been changed to an interfacenetwork.NetworkI. This change improves code maintainability and encapsulation.40-52: The
submitProposalfunction's signature has been updated to acceptnetwork.ValidatorIinstead of*network.Validator. The function now usesval.GetClientCtx()andval.GetAddress().String()instead ofval.ClientCtxandval.Address.String()respectively. This change improves code maintainability and encapsulation.75-76: The
TestQueryDepositsWithInitialDepositfunction now usess.network.GetValidators()[0]instead ofs.network.Validators[0]. This change improves code maintainability and encapsulation.95-96: The
TestQueryProposalAfterVotingPeriodfunction now usess.network.GetValidators()[0]instead ofs.network.Validators[0]. This change improves code maintainability and encapsulation.139-154: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [130-146]
The
queryDepositsfunction's signature has been updated to acceptnetwork.ValidatorIinstead of*network.Validator. This change improves code maintainability and encapsulation.
- 139-154: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [148-163]
The
queryDepositfunction's signature has been updated to acceptnetwork.ValidatorIinstead of*network.Validator. This change improves code maintainability and encapsulation.tests/e2e/distribution/withdraw_all_suite.go (1)
- 24-28: The
networkfield inWithdrawAllTestSuitestruct has been replaced withnetwork.NetworkIinterface type. This is a good practice as it allows for easy swapping and use of different implementations.tests/e2e/client/grpc/cmtservice/service_test.go (4)
26-31: The
networkfield in theE2ETestSuitestruct has been changed from*network.Networktonetwork.NetworkI. This change allows for greater flexibility in the types of network that can be used in the test suite, as any type that implements theNetworkIinterface can now be used.50-50: The
GetValidatorsmethod is used to retrieve the validators from the network. This is a good use of the newNetworkIinterface, as it abstracts away the details of how the validators are stored and retrieved.234-250: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [207-261]
The
GetValidatorsmethod is used to retrieve the validators from the network, and theGetClientCtx,GetAPIAddress, andGetPubKeymethods are used to retrieve information about the validators. This is a good use of the newValidatorIinterface, as it abstracts away the details of how the validator information is stored and retrieved.
- 234-250: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [193-261]
The
GetClientCtxmethod is used to retrieve the client context from the validator, which is then used to unmarshal the response from the server. This is a good use of the newValidatorIinterface, as it abstracts away the details of how the client context is stored and retrieved.server/api/server_test.go (1)
- 40-45: The
networkfield in theGRPCWebTestSuitestruct has been updated to use thenetwork.NetworkIinterface instead of thenetwork.Networktype. This change allows for greater flexibility in the implementation of thenetworkfield, as any type that implements thenetwork.NetworkIinterface can be used.tests/integration/server/grpc/server_test.go (1)
- 33-38: The
networkfield in theIntegrationTestSuitestruct has been changed from*network.Networktonetwork.NetworkI. This change is part of the refactor to use interfaces for better encapsulation and abstraction.tests/e2e/tx/benchmarks_test.go (7)
24-27: The
networkfield in theE2EBenchmarkSuitestruct has been changed to thenetwork.NetworkIinterface type. This change allows for better encapsulation and flexibility in using different network implementations.44-55: The
networkandvalobjects are now accessed through theGetValidators()andGetClientCtx()methods, respectively, instead of directly. This change improves code maintainability and encapsulation.95-116: The
networkobject is created using thenetwork.New()function, and thevalobject is accessed through theGetValidators()method. ThequeryClientis created using theval.GetClientCtx()method. These changes improve code maintainability and encapsulation.119-137: The
valobject is accessed through theGetClientCtx()andGetAddress()methods, and thetxResobject is unmarshalled using theval.GetClientCtx().Codec.UnmarshalJSON()method. These changes improve code maintainability and encapsulation.142-151: The
valobject is accessed through theGetClientCtx()method, and therespobject is created using thecli.GetTxResponse()function withs.networkandval.GetClientCtx()as arguments. These changes improve code maintainability and encapsulation.158-174: The
valobject is accessed through theGetValidators()andGetClientCtx()methods, and thetxBuilderobject is created using theval.GetClientCtx().TxConfig.NewTxBuilder()method. These changes improve code maintainability and encapsulation.178-190: The
valobject is accessed through theGetClientCtx(),GetMoniker(), andGetAddress()methods, and thetxFactoryobject is created using theval.GetClientCtx().ChainID,val.GetClientCtx().Keyring, andval.GetClientCtx().TxConfigfields. These changes improve code maintainability and encapsulation.testutil/network/network.go (7)
- 6-11: These imports were flagged as unused in a previous review. If they are still not used, they should be removed.
- "encoding/json" - "errors" - "fmt" - "net/url" - "os" - "os/signal"
- 16-23: These imports were flagged as unused in a previous review. If they are still not used, they should be removed.
- "testing" - "time" - dbm "github.com/cosmos/cosmos-db" - "github.com/spf13/cobra" - "cosmossdk.io/core/address" - "cosmossdk.io/depinject"
- 46-51: These imports were flagged as unused in a previous review. If they are still not used, they should be removed.
- cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" - "github.com/cosmos/cosmos-sdk/runtime" - "github.com/cosmos/cosmos-sdk/server" - srvconfig "github.com/cosmos/cosmos-sdk/server/config" - servertypes "github.com/cosmos/cosmos-sdk/server/types" - "github.com/cosmos/cosmos-sdk/testutil"
268-273: The Logger interface is defined but not used in the provided hunks. Ensure that it is used elsewhere in the code.
280-282: Ensure that the Logger interface is implemented by the
testing.TandCLILoggertypes elsewhere in the code.302-308: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [305-309]
The
Newfunction creates a new Network. Ensure that the lock is released at some point to avoid potential deadlocks.
- 685-691: The
GetValidatorsfunction is a new addition and seems to be correctly implemented. It returns a slice ofValidatorIinterface instances.tests/e2e/gov/tx.go (1)
- 26-30: The
networkfield in theE2ETestSuitestruct has been changed from*network.Networktonetwork.NetworkI. This change reflects the shift from a concrete type to an interface, which allows for greater flexibility and easier testing.tests/e2e/tx/service_test.go (8)
45-45: The
networkfield in theE2ETestSuitestruct is now of typeNetworkIinstead of*network.Network. This change is likely to improve modularity and testability by allowing different implementations of theNetworkIinterface to be used in tests. Ensure that all methods that were previously called onnetworkare available in theNetworkIinterface.63-78: The
complex_functionnow uses getter methods to accessValidatorproperties instead of accessing them directly. This is a good practice as it encapsulates the properties of theValidatorand allows for additional logic to be added in the getter methods if needed in the future. However, ensure that these getter methods are implemented correctly and return the expected values.87-89: The
complex_functionnow uses theClientCtx'sCodecto unmarshal JSON. This is a good practice as it allows for different codecs to be used depending on the context. However, ensure that theCodecis correctly set in theClientCtx.205-211: The
complex_functionnow uses theClientCtx'sTxConfigto encode transactions. This is a good practice as it allows for different transaction configurations to be used depending on the context. However, ensure that theTxConfigis correctly set in theClientCtx.227-235: The
complex_functionnow uses theClientCtx'sCodecto marshal and unmarshal JSON. This is a good practice as it allows for different codecs to be used depending on the context. However, ensure that theCodecis correctly set in theClientCtx.566-569: The
complex_functionnow uses theClientCtx'sKeyringinstead of accessing it directly. This is a good practice as it encapsulates theKeyringand allows for additional logic to be added in the getter method if needed in the future. However, ensure that the getter method is implemented correctly and returns the expected value.898-907: The
complex_functionnow uses theClientCtx'sCodecto marshal and unmarshal JSON. This is a good practice as it allows for different codecs to be used depending on the context. However, ensure that theCodecis correctly set in theClientCtx.1005-1009: The
complex_functionnow uses theClientCtx'sTxConfigto encode transactions and theClientCtx'sLegacyAminoto unmarshal transactions. This is a good practice as it allows for different transaction configurations and codecs to be used depending on the context. However, ensure that theTxConfigandLegacyAminoare correctly set in theClientCtx.tests/e2e/authz/grpc.go (16)
19-24: The refactoring of direct access to fields of
networkandValidatorobjects to method calls on the newNetworkIandValidatorIinterfaces is a good practice for encapsulation and maintainability.33-38: The refactoring of direct access to fields of
networkandValidatorobjects to method calls on the newNetworkIandValidatorIinterfaces is a good practice for encapsulation and maintainability.45-63: The refactoring of direct access to fields of
networkandValidatorobjects to method calls on the newNetworkIandValidatorIinterfaces is a good practice for encapsulation and maintainability.71-80: The refactoring of direct access to fields of
networkandValidatorobjects to method calls on the newNetworkIandValidatorIinterfaces is a good practice for encapsulation and maintainability.88-93: The refactoring of direct access to fields of
networkandValidatorobjects to method calls on the newNetworkIandValidatorIinterfaces is a good practice for encapsulation and maintainability.98-104: The refactoring of direct access to fields of
networkandValidatorobjects to method calls on the newNetworkIandValidatorIinterfaces is a good practice for encapsulation and maintainability.108-121: The refactoring of direct access to fields of
networkandValidatorobjects to method calls on the newNetworkIandValidatorIinterfaces is a good practice for encapsulation and maintainability.131-137: The refactoring of direct access to fields of
networkandValidatorobjects to method calls on the newNetworkIandValidatorIinterfaces is a good practice for encapsulation and maintainability.141-147: The refactoring of direct access to fields of
networkandValidatorobjects to method calls on the newNetworkIandValidatorIinterfaces is a good practice for encapsulation and maintainability.161-167: The refactoring of direct access to fields of
networkandValidatorobjects to method calls on the newNetworkIandValidatorIinterfaces is a good practice for encapsulation and maintainability.173-176: The refactoring of direct access to fields of
networkandValidatorobjects to method calls on the newNetworkIandValidatorIinterfaces is a good practice for encapsulation and maintainability.183-203: The refactoring of direct access to fields of
networkandValidatorobjects to method calls on the newNetworkIandValidatorIinterfaces is a good practice for encapsulation and maintainability.212-218: The refactoring of direct access to fields of
networkandValidatorobjects to method calls on the newNetworkIandValidatorIinterfaces is a good practice for encapsulation and maintainability.224-227: The refactoring of direct access to fields of
networkandValidatorobjects to method calls on the newNetworkIandValidatorIinterfaces is a good practice for encapsulation and maintainability.234-254: The refactoring of direct access to fields of
networkandValidatorobjects to method calls on the newNetworkIandValidatorIinterfaces is a good practice for encapsulation and maintainability.263-269: The refactoring of direct access to fields of
networkandValidatorobjects to method calls on the newNetworkIandValidatorIinterfaces is a good practice for encapsulation and maintainability.tests/e2e/group/suite.go (2)
27-31: The
networkfield in theE2ETestSuitestruct has been changed from*network.Networktonetwork.NetworkI. This change allows for better abstraction and flexibility in using different network implementations.257-261: The
networkandValidatorobjects are now accessed through theNetworkIandValidatorIinterfaces. This improves code maintainability and encapsulation by preventing direct access to the fields of these objects.- s.Require().NoError(val.ClientCtx.Codec.UnmarshalJSON(out.Bytes(), &txResp), out.String()) + s.Require().NoError(val.GetClientCtx().Codec.UnmarshalJSON(out.Bytes(), &txResp), out.String())[
tests/e2e/auth/suite.go (3)
43-43: The previous comment about the unused
networkfield still stands. If it's not being used, consider removing it to improve code readability.56-56: The previous comment about the hardcoded mnemonic "newAccount" still stands. If this is sensitive information, consider moving it to a secure storage or configuration file.
686-686: The error from
authclitestutil.TxSignExecis not being handled. Consider adding error handling to prevent potential issues.- signedTx, err := authclitestutil.TxSignExec(clientCtx, val1.GetAddress(), unsignedTxFile.Name()) + signedTx, err := authclitestutil.TxSignExec(clientCtx, val1.GetAddress(), unsignedTxFile.Name()) + s.Require(). </blockquote></details> <details><summary>tests/e2e/authz/tx.go (36)</summary><blockquote> * 37-40: The `network` and `grantee` fields are not initialized in the struct. Ensure that they are properly initialized before use. * 49-55: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [52-56] Ensure that the `GetValidators()` method returns at least one validator to avoid an index out of range error. * 58-64: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [61-67] Ensure that the `MsgSubmitLegacyProposal` function is correctly handling all the arguments and that the proposal type is valid. * 73-78: Ensure that the `CreateGrant` function is correctly handling all the arguments and that the grantee address is valid. * 86-88: Ensure that the `CheckTxCode` function is correctly handling the transaction hash and that the transaction hash is valid. * 127-140: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [134-142] Ensure that the `createAccount` function is correctly handling all the arguments and that the keyring is properly initialized. * 144-153: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [147-154] Ensure that the `msgSendExec` function is correctly handling all the arguments and that the account address is valid. * 159-162: Ensure that the `SubmitTestTx` function is correctly handling all the arguments and that the transaction message is valid. * 181-197: Ensure that the `CreateGrant` function is correctly handling all the arguments and that the grantee address is valid. * 221-238: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [224-240] Ensure that the `CreateGrant` function is correctly handling all the arguments and that the grantee address is valid. * 245-248: Ensure that the `WriteToNewTempFile` function is correctly handling all the arguments and that the vote transaction is valid. * 308-314: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [311-316] Ensure that the `ExecTestCLICmd` function is correctly handling all the arguments and that the command is valid. * 320-322: Ensure that the `CheckTxCode` function is correctly handling the transaction hash and that the transaction hash is valid. * 317-342: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [327-345] Ensure that the `CreateGrant` function is correctly handling all the arguments and that the grantee address is valid. * 348-358: Ensure that the `SubmitTestTx` function is correctly handling all the arguments and that the transaction message is valid. * 418-424: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [421-426] Ensure that the `ExecTestCLICmd` function is correctly handling all the arguments and that the command is valid. * 436-438: Ensure that the `CheckTxCode` function is correctly handling the transaction hash and that the transaction hash is valid. * 433-459: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [442-460] Ensure that the `CreateGrant` function is correctly handling all the arguments and that the grantee address is valid. * 466-468: Ensure that the `MsgSend` struct is correctly initialized and that the account addresses are valid. * 477-480: Ensure that the `SubmitTestTx` function is correctly handling all the arguments and that the transaction message is valid. * 494-497: Ensure that the `SubmitTestTx` function is correctly handling all the arguments and that the transaction message is valid. * 515-520: Ensure that the `ExecTestCLICmd` function is correctly handling all the arguments and that the command is valid. * 528-530: Ensure that the `ExecTestCLICmd` function is correctly handling all the arguments and that the command is valid. * 542-560: Ensure that the `CreateGrant` function is correctly handling all the arguments and that the grantee address is valid. * 568-570: Ensure that the `MsgDelegate` struct is correctly initialized and that the account addresses are valid. * 622-628: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [625-630] Ensure that the `ExecTestCLICmd` function is correctly handling all the arguments and that the command is valid. * 635-637: Ensure that the `CheckTxCode` function is correctly handling the transaction hash and that the transaction hash is valid. * 632-653: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [640-658] Ensure that the `CreateGrant` function is correctly handling all the arguments and that the grantee address is valid. * 661-663: Ensure that the `MsgDelegate` struct is correctly initialized and that the account addresses are valid. * 689-695: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [692-697] Ensure that the `ExecTestCLICmd` function is correctly handling all the arguments and that the command is valid. * 702-704: Ensure that the `CheckTxCode` function is correctly handling the transaction hash and that the transaction hash is valid. * 699-721: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [707-725] Ensure that the `CreateGrant` function is correctly handling all the arguments and that the grantee address is valid. * 733-735: Ensure that the `ExecTestCLICmd` function is correctly handling all the arguments and that the command is valid. * 749-767: Ensure that the `CreateGrant` function is correctly handling all the arguments and that the grantee address is valid. * 774-776: Ensure that the `MsgDelegate` struct is correctly initialized and that the account addresses are valid. * 779-781: Ensure that the `SubmitTestTx </blockquote></details></blockquote></details> </details>
There was a problem hiding this comment.
Refactor looks good.
@coderabbitai did a great job in review. Only few unused fields needs to be removed as suggested by the bot.
the bot was wrong, most of the review from it was pointless |
Agreed. But this comment is because, particularly I noticed here. The clientCtx defined here is just used in the following line L247, but it could be just used as |
| // Validator defines an in-process CometBFT validator node. Through this object, | ||
| // a client can make RPC and API calls and interact with any client command | ||
| // or handler. | ||
| type Validator struct { |
There was a problem hiding this comment.
If the goal is to limit the usage of the fields with getters, why those fields are still public?
tac0turtle
force-pushed
the
marko/18145
branch
from
425f887 to
cb080d8
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (3)
- testutil/network/network.go (10 hunks)
- testutil/network/util.go (7 hunks)
- testutil/network/validator.go (1 hunks)
Additional comments: 17
testutil/network/util.go (5)
33-37: The function
startInProcessnow accepts a pointer toValidator. Ensure that all calls to this function have been updated to pass a pointer instead of a value.47-48: The
cfg.AppConstructoris now called with a pointer toval. This could potentially modify thevalobject within theAppConstructor. Ensure that this is the intended behavior.79-81: The
val.RPCAddresshas been changed toval.rPCAddressandval.RPCClienttoval.rPCClient. Ensure that these changes are reflected throughout the codebase.84-91: The
val.APIAddresshas been changed toval.aPIAddressandval.ClientCtxtoval.clientCtx. Ensure that these changes are reflected throughout the codebase.134-146: The
val.Ctxhas been changed toval.ctx. Ensure that these changes are reflected throughout the codebase.testutil/network/validator.go (2)
48-48: Good practice to ensure that the
Validatorstruct implements theValidatorIinterface.50-84: The getter methods are correctly implemented and provide access to the private fields of the
Validatorstruct.testutil/network/network.go (10)
6-11: The removal of these imports indicates that the associated dependencies are no longer required in this file. Ensure that this does not break any functionality.
16-23: The removal of these imports indicates that the associated dependencies are no longer required in this file. Ensure that this does not break any functionality.
46-51: The removal of these imports indicates that the associated dependencies are no longer required in this file. Ensure that this does not break any functionality.
268-273: The Logger interface is introduced to provide a standardized way of logging for the network. This is a good practice as it allows for different logging implementations to be used interchangeably.
280-283: The Logger interface is implemented by *testing.T and *CLILogger. This is a good practice as it allows for different logging implementations to be used interchangeably.
305-305: The New function now returns an interface type NetworkI instead of a concrete type. This is a good practice as it allows for different implementations of the NetworkI interface to be used interchangeably.
567-577: The Validator struct has been refactored, with changes to field names and types. Ensure that these changes are reflected in all parts of the codebase where the Validator struct is used.
651-651: The Validator struct is now accessed through the Validators slice of the Network struct. Ensure that this change is reflected in all parts of the codebase where the Validator struct is used.
685-691: The GetValidators function has been added to the Network struct. This is a good practice as it provides a standardized way of accessing the Validators slice.
708-708: The Validator struct is now accessed through the Validators slice of the Network struct. Ensure that this change is reflected in all parts of the codebase where the Validator struct is used.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- tests/e2e/client/grpc/cmtservice/service_test.go (10 hunks)
Additional comments: 4
tests/e2e/client/grpc/cmtservice/service_test.go (4)
26-31: The type of
networkfield inE2ETestSuitestruct has been changed from*network.Networktonetwork.NetworkI. Ensure that all methods and properties ofnetwork.Networkthat were being used are available innetwork.NetworkI.47-51: The
networkfield is being initialized withnetwork.Newandnetwork.WaitForNextBlockis being called. Ensure that these methods are available and behave as expected in the newnetwork.NetworkIinterface.234-250: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [236-261]
In these lines, the
GetValidatorsmethod is being called on thenetworkfield. Ensure that this method is available and behaves as expected in the newnetwork.NetworkIinterface.
- 234-250: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [236-261]
In these lines, the
GetValidatorsmethod is being called on thenetworkfield and the returned validators are being used. Ensure that the validators returned by this method in the newnetwork.NetworkIinterface have the same properties and methods as the validators in the oldnetwork.Networkstruct.
Description
Closes: #18145
make testutil.Network an interface to easily swap out and use different implementations
the file interface.go will change to network.go in a follow up pr where we separate the local network items into its own go.mod
this pr does not make it so that the new function can decide the environment to test with, this will be in a follow up pr once starship has integrated the interface for tests to use
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
Summary by CodeRabbit