fix: increase validators list pagination in query #85
Conversation
WalkthroughThe pull request introduces several modifications across multiple files, primarily focusing on increasing the pagination limit for fetching validators from the Cosmos API. The limit has been adjusted from Changes
Possibly related PRs
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #85 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 65 65
Lines 2736 3351 +615
==========================================
+ Hits 2736 3351 +615 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
cmd/cosmos-validators-exporter_test.go (1)
Line range hint
1-82: Consider restructuring tests for better maintainability and performanceThe current test structure has several areas for improvement:
- Heavy reliance on panic recovery for validation could be replaced with more explicit error handling
- Tests are explicitly marked as non-parallel, which could impact test execution time
- Global state modifications (os.Args) make tests hard to parallelize
Consider these improvements:
- Instead of using
os.Argsandmain(), create a testable function that accepts config path as parameter- Use error returns instead of panics for validation failures
- Implement proper test cleanup to allow parallel execution
Example approach:
func validateConfig(configPath string) error { // Move validation logic here return nil } func TestValidateConfig(t *testing.T) { t.Parallel() tests := []struct { name string configPath string wantErr bool wantWarns bool }{ {"valid config", "../assets/config-valid.toml", false, false}, {"config with warnings", "../assets/config-with-warnings.toml", false, true}, {"invalid config", "../assets/config-invalid.toml", true, false}, } for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() err := validateConfig(tt.configPath) if tt.wantErr { require.Error(t, err) } else { require.NoError(t, err) } }) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
.golangci.yml(0 hunks)cmd/cosmos-validators-exporter_test.go(1 hunks)pkg/app_test.go(2 hunks)
💤 Files with no reviewable changes (1)
- .golangci.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/app_test.go
🔇 Additional comments (1)
cmd/cosmos-validators-exporter_test.go (1)
47-56: Verify test configuration files
Since these tests rely on configuration files in the "../assets/" directory, we should verify their existence and content.
✅ Verification successful
Test configuration files are valid and sufficient
The test configuration files exist and contain appropriate validator configurations. While the files don't explicitly contain pagination settings (as these are handled in the code), the code search reveals that pagination is properly implemented in the codebase:
- Validator queries use a limit of 10000 (
pagination.limit=10000) - Delegation and unbonding queries use appropriate pagination
- Test files properly mock these paginated endpoints
The test files config-valid.toml, config-with-warnings.toml, and config-invalid.toml are present and contain the necessary validator configurations to exercise these scenarios.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify test configuration files exist and contain appropriate test data
# Check if config files exist
echo "Checking config files existence..."
for config in config-valid.toml config-with-warnings.toml config-invalid.toml; do
if [ -f "assets/$config" ]; then
echo "Found: assets/$config"
# Check for pagination settings
echo "Content of $config:"
cat "assets/$config"
else
echo "Missing: assets/$config"
fi
done
# Search for pagination-related settings in all TOML files
echo -e "\nChecking pagination settings in all TOML files..."
rg -l --type toml "pagination|limit|page"
Length of output: 4311
Script:
#!/bin/bash
# Let's check the code for pagination-related changes and configurations
# Search for pagination-related code changes
echo "Searching for pagination-related code..."
rg -n "pagination|limit|page" --type go
# Look for validator-related configuration or constants
echo -e "\nSearching for validator-related constants or configurations..."
rg -n "validator.*limit|page.*size|max.*validator" --type go
# Check for any test files that might contain pagination tests
echo -e "\nSearching for pagination-related test cases..."
rg -n "Test.*Page|Test.*Limit|Test.*Pagination" --type go
Length of output: 6927
| func TestValidateConfigWithWarnings(_ *testing.T) { | ||
| os.Args = []string{"cmd", "validate-config", "--config", "../assets/config-with-warnings.toml"} | ||
| main() | ||
| assert.True(t, true) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance test coverage with explicit assertions
Both TestValidateConfigWithWarnings and TestValidateConfigValid lack assertions to verify the expected behavior. Simply checking that the code doesn't panic might miss subtle bugs where the code runs but produces incorrect results.
Consider adding assertions to verify:
- The config is loaded correctly with expected values
- For
TestValidateConfigWithWarnings, verify that appropriate warnings are logged - For
TestValidateConfigValid, verify the validation results
Example refactor:
func TestValidateConfigWithWarnings(t *testing.T) {
+ // Capture logs or use a mock logger
+ logs := captureTestLogs(t)
os.Args = []string{"cmd", "validate-config", "--config", "../assets/config-with-warnings.toml"}
main()
+ // Verify expected warnings were logged
+ require.Contains(t, logs, "expected warning message")
}
func TestValidateConfigValid(t *testing.T) {
+ // Setup test expectations
+ expectedConfig := Config{...} // Add expected values
os.Args = []string{"cmd", "validate-config", "--config", "../assets/config-valid.toml"}
main()
+ // Verify config was loaded with correct values
+ require.Equal(t, expectedConfig, actualConfig)
}Also applies to: 53-56
Related: #84
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tokensfield type in theValidatorstruct for better precision in token representation.