Skip to content

Conversation

@zbchi
Copy link
Contributor

@zbchi zbchi commented Dec 22, 2025

Summary

Improve unit test coverage for the cluster package and fix a boundary check bug in argument matcher.

Changes

  • Add unit tests for cluster package functionality:

    • cluster/cluster/:
      • interceptor_invoker_test.go: Interceptor chain building and management
      • mock_test.go: Mock invoker and cluster implementations
    • cluster/loadbalance/:
      • aliasmethod/alias_method_test.go: Alias method picker for weighted load balancing
      • iwrr/iwrr_test.go: Interleaved weighted round-robin queue operations
      • util_test.go: Weight calculation with warmup and priority handling
    • cluster/metrics/:
      • local_metrics_test.go: Local metrics storage and concurrent access
      • utils_test.go: Invoker and instance key generation utilities
    • cluster/router/:
      • condition/matcher/argument_test.go: Argument condition matcher
      • condition/matcher/attachment_test.go: Attachment condition matcher
      • condition/matcher/base_test.go: Base condition matcher and sorting
      • options_test.go: Router configuration options
      • script/instance/instances_pool_test.go: Script router instance pool
    • cluster/utils/:
      • adaptivesvc_test.go: Adaptive service error handling
      • version_test.go: Version parsing and comparison
  • Fix bug in cluster/router/condition/matcher/argument.go:

    • Fix boundary check from index > len(...) to index >= len(...) to prevent index out of range panic

Checklist

  • I confirm the target branch is develop
  • Code has passed local testing
  • I have added tests that prove my fix is effective or that my feature works

@codecov-commenter
Copy link

codecov-commenter commented Dec 22, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 45.48%. Comparing base (ffc02ca) to head (c5b9726).
⚠️ Report is 29 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3135      +/-   ##
===========================================
+ Coverage    37.04%   45.48%   +8.44%     
===========================================
  Files          460      460              
  Lines        32997    32997              
===========================================
+ Hits         12224    15010    +2786     
+ Misses       19547    16726    -2821     
- Partials      1226     1261      +35     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds comprehensive unit test coverage for the cluster package and fixes a critical boundary check bug in the argument matcher that could cause index out of bounds panics.

Key changes:

  • Fixed boundary check in ArgumentConditionMatcher from index > len(...) to index >= len(...) to prevent array access errors
  • Added extensive unit tests covering cluster operations, load balancing algorithms, routing conditions, and metrics
  • Standardized test IP addresses from 192.168.1.x to 127.0.0.1 with different ports in P2C load balancer tests

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
cluster/router/condition/matcher/argument.go Fixed boundary check bug to prevent index out of bounds when accessing invocation arguments
cluster/utils/version_test.go Tests for version parsing, comparison, and validation with various formats
cluster/utils/adaptivesvc_test.go Tests for adaptive service error detection and handling
cluster/router/script/instance/instances_pool_test.go Tests for script router instance pool including wrapper lifecycle and JavaScript instance retrieval
cluster/router/options_test.go Tests for router configuration options builders (contains tests validating buggy behavior)
cluster/router/condition/matcher/base_test.go Tests for base condition matcher functionality and value extraction
cluster/router/condition/matcher/attachment_test.go Tests for attachment-based condition matching with various key formats
cluster/router/condition/matcher/argument_test.go Tests for argument-based condition matching including boundary cases
cluster/metrics/utils_test.go Tests for invoker and instance key generation utilities
cluster/metrics/local_metrics_test.go Tests for local metrics storage with concurrency validation
cluster/loadbalance/util_test.go Tests for weight calculation including warmup and priority handling
cluster/loadbalance/p2c/loadbalance_test.go Updated test IP addresses to use localhost with different ports
cluster/loadbalance/iwrr/iwrr_test.go Tests for interleaved weighted round-robin queue operations and GCD calculations
cluster/loadbalance/aliasmethod/alias_method_test.go Tests for alias method weighted selection algorithm
cluster/cluster/mock_test.go Tests for mock invoker and cluster implementations
cluster/cluster/interceptor_invoker_test.go Tests for interceptor chain building and invocation order

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@AlexStocks
Copy link
Contributor

please review the copilot's comment.

@zbchi
Copy link
Contributor Author

zbchi commented Dec 22, 2025

please review the copilot's comment.

ok

@sonarqubecloud
Copy link

@zbchi zbchi closed this Dec 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants