-
Notifications
You must be signed in to change notification settings - Fork 610
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
chore: use NotEmpty(x)
instead NotZero(len(x))
#6076
Conversation
WalkthroughWalkthroughThe recent updates involve modifying assertions in test suites related to interchain account operations and upgrades. Specifically, assertions checking Changes
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 (
|
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.
swift and clean, thanks for the clean up!
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 Status
Configuration used: .coderabbit.yml
Files selected for processing (6)
- e2e/tests/interchain_accounts/base_test.go (4 hunks)
- e2e/tests/interchain_accounts/localhost_test.go (6 hunks)
- e2e/tests/interchain_accounts/params_test.go (1 hunks)
- e2e/tests/interchain_accounts/query_test.go (1 hunks)
- e2e/tests/interchain_accounts/upgrades_test.go (1 hunks)
- e2e/tests/upgrades/genesis_test.go (1 hunks)
Additional Context Used
Path-based Instructions (6)
e2e/tests/interchain_accounts/query_test.go (3)
Pattern
**/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
e2e/**/*
: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern
**/*_test.go
: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"e2e/tests/upgrades/genesis_test.go (3)
Pattern
**/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
e2e/**/*
: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern
**/*_test.go
: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"e2e/tests/interchain_accounts/params_test.go (3)
Pattern
**/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
e2e/**/*
: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern
**/*_test.go
: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"e2e/tests/interchain_accounts/upgrades_test.go (3)
Pattern
**/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
e2e/**/*
: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern
**/*_test.go
: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"e2e/tests/interchain_accounts/localhost_test.go (3)
Pattern
**/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
e2e/**/*
: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern
**/*_test.go
: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"e2e/tests/interchain_accounts/base_test.go (3)
Pattern
**/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
e2e/**/*
: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern
**/*_test.go
: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
Additional comments not posted (14)
e2e/tests/interchain_accounts/query_test.go (1)
69-69
: The change fromNotZero(len(hostAccount))
toNotEmpty(hostAccount)
improves the readability and expressiveness of the test assertion. This is a positive change that makes the test more intuitive.e2e/tests/upgrades/genesis_test.go (1)
118-118
: The change fromNotZero(len(hostAccount))
toNotEmpty(hostAccount)
enhances the readability and expressiveness of the test assertion, making it more intuitive. Good improvement.e2e/tests/interchain_accounts/params_test.go (1)
158-158
: Changing the assertion fromNotZero(len(hostAccount))
toNotEmpty(hostAccount)
is a good practice for improving test readability and expressiveness. This makes the test more intuitive.e2e/tests/interchain_accounts/upgrades_test.go (1)
83-83
: The replacement ofNotZero(len(hostAccount))
withNotEmpty(hostAccount)
in the assertion improves test readability and expressiveness, aligning with best practices for clear and intuitive testing.e2e/tests/interchain_accounts/localhost_test.go (6)
125-125
: The change fromNotZero(len(interchainAccAddress))
toNotEmpty(interchainAccAddress)
improves readability and directly conveys the intention to check for non-empty strings. This is a positive change that aligns with best practices for writing clear and concise tests.
139-139
: The replacement ofNotZero(len(interchainAccAddress))
withNotEmpty(interchainAccAddress)
here is consistent with the previous change and maintains the improved readability and directness in expressing the test's intention. Good job on maintaining consistency across test cases.
280-280
: Again, the use ofNotEmpty(interchainAccAddress)
instead ofNotZero(len(interchainAccAddress))
is observed here. This change is consistent across the file and enhances the clarity of the test assertions. It's great to see uniformity in the approach to testing across different test cases.
294-294
: The application ofNotEmpty(interchainAccAddress)
in place ofNotZero(len(interchainAccAddress))
is consistently applied here as well. This consistency in making the test assertions more readable and expressive is commendable.
410-410
: The use ofNotEmpty(interchainAccAddress)
for asserting the non-emptiness ofinterchainAccAddress
is consistently applied here, further demonstrating attention to readability and clarity in the test assertions. This consistency is key to maintaining a clean and understandable codebase.
422-422
: The consistent replacement ofNotZero(len(interchainAccAddress))
withNotEmpty(interchainAccAddress)
across all relevant test cases, as seen here, is a good practice. It ensures that the test assertions are both clear and expressive, making the tests easier to understand at a glance.e2e/tests/interchain_accounts/base_test.go (4)
93-93
: The change fromNotZero(len(hostAccount))
toNotEmpty(hostAccount)
in this test case is a positive improvement. It directly checks for non-empty strings, which is more readable and expressive. This change aligns with best practices for writing clear and concise tests.
192-192
: ReplacingNotZero(len(hostAccount))
withNotEmpty(hostAccount)
here is consistent with the changes made in other test cases. This consistency in improving the readability and expressiveness of test assertions is commendable.
290-290
: The use ofNotEmpty(hostAccount)
instead ofNotZero(len(hostAccount))
for asserting the non-emptiness ofhostAccount
is consistently applied here. This change enhances the clarity of the test assertions and is a good practice for maintaining a clean and understandable codebase.
470-470
: The consistent replacement ofNotZero(len(hostAccount))
withNotEmpty(hostAccount)
across all relevant test cases, as seen here, ensures that the test assertions are both clear and expressive. This consistency is key to maintaining readability and understandability in the test suite.
Quality Gate passed for 'ibc-go'Issues Measures |
Description
ref: #6067 (comment)
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/
).godoc
comments.Files changed
in the GitHub PR explorer.SonarCloud Report
in the comment section below once CI passes.Summary by CodeRabbit