test(e2e): temporarily disable rate limiter e2e test #2463
Conversation
lumtis
added
no-changelog
lumtis
requested review from
brewmaster012,
kingpinXD,
ws4charlie,
skosito,
swift1337 and
fbac
as code owners
WalkthroughThe Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
cmd/zetae2e/local/local.go (1)
281-289: LGTM! Consider adding more context to the TODO comments.The comments are clear and provide the necessary context for disabling the test. The TODO comments properly reference the relevant GitHub issues.
Consider adding more context to the TODO comments to specify what needs to be fixed or defined.
- // TODO: fix the error causing the test failure + // TODO: Investigate and fix the underlying issue causing the TestRateLimiterName test failure - // TODO: define proper threshold for test assertion + // TODO: Define proper threshold for rate limiting in the TestRateLimiterName test assertion
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- cmd/zetae2e/local/local.go (1 hunks)
Additional context used
Path-based instructions (1)
cmd/zetae2e/local/local.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
cmd/zetae2e/local/local.go (1)
281-289: Track TODOs in the issue tracker.The comments provide clear context for why the
TestRateLimiterNametest is disabled and link to relevant issues. However, it is recommended to track TODOs in the issue tracker rather than in the code to ensure they are not overlooked.- // TODO: fix the error causing the test failure - // https://github.com/zeta-chain/node/issues/2424 - // TODO: define proper threshold for test assertion - // https://github.com/zeta-chain/node/issues/2090 + // See issue #2424 for fixing the error causing the test failure. + // See issue #2090 for defining a proper threshold for test assertion.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- cmd/zetae2e/local/local.go (1 hunks)
Additional context used
Path-based instructions (1)
cmd/zetae2e/local/local.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- cmd/zetae2e/local/local.go (1 hunks)
Additional context used
Path-based instructions (1)
cmd/zetae2e/local/local.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
Additional comments not posted (1)
cmd/zetae2e/local/local.go (1)
281-288: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
adminTestRoutineare updated accordingly to match the new parameter list.Verification successful
Verified: The function calls to
adminTestRoutinehave been updated accordingly to match the new parameter list.
- The function call and definition are consistent with each other.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `adminTestRoutine` match the new parameter list. # Test: Search for the function usage. Expect: Only occurances of the new parameter list. rg --type go $'adminTestRoutine'Length of output: 238
Script:
#!/bin/bash # Retrieve the function definition of adminTestRoutine from cmd/zetae2e/local/admin.go rg --type go -A 10 $'func adminTestRoutine' cmd/zetae2e/local/admin.go # Retrieve the call to adminTestRoutine from cmd/zetae2e/local/local.go rg --type go -A 5 $'adminTestRoutine' cmd/zetae2e/local/local.goLength of output: 662
|
Closing as it has been fixed |
Description
Temporarily disable the rate limiter E2E test for letting other admin test pass.
Issue for the bug tracked: #2424
Disabling the test for now is a easier workaround as the rate limiter E2E test doesn't properly test the functionality: #2090 and we need the other test to be testable directly in the CI to simplify the testing process for the coming release
Summary by CodeRabbit
Bug Fixes
Chores