[test] Add tests for server.isRateLimitToolResult and server.extractRateLimitErrorText - #9643
Conversation
…rrorText Both functions in internal/server/rate_limit.go were previously only exercised indirectly via circuit_breaker_test.go integration tests. This commit adds direct table-driven unit tests covering: - extractRateLimitErrorText: non-map input (returns default message), map without content, map with empty content, map with text content, map with non-text content type, map with multiple content items - isRateLimitToolResult: non-map/nil input, isError=false with rate limit text, missing isError field, isError=true with various rate limit phrases (rate limit exceeded, secondary rate limit, too many requests, rate limit 403), isError=true with reset time in text (verifies non-zero time.Time returned), empty content Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds direct unit coverage for rate-limit result detection and error-text extraction.
Changes:
- Tests invalid inputs, error flags, rate-limit phrases, and reset timestamps.
- Tests text extraction across content variants.
Show a summary per file
| File | Description |
|---|---|
internal/server/rate_limit_test.go |
Adds rate-limit helper unit tests. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
internal/server/rate_limit_test.go:185
- This test name is already declared in
internal/server/circuit_breaker_test.go:203, which makes theservertest package fail to compile. Rename this additional branch-focused test (or merge these cases into the existing table).
func TestIsRateLimitToolResult(t *testing.T) {
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Medium
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Fixed in commit I investigated the two failing jobs and addressed the failure by renaming the duplicate test function in Validation run:
|
Test Coverage Improvement:
isRateLimitToolResultandextractRateLimitErrorTextFunction Analyzed
internal/serverisRateLimitToolResult,extractRateLimitErrorTextcircuit_breaker_test.gointegration testsWhy These Functions?
isRateLimitToolResultcontains several code paths that determine whether a raw tool result represents a GitHub rate-limit error and parses an optional reset timestamp.extractRateLimitErrorTextextracts the human-readable error message for agents. Both were only reachable through higher-level circuit-breaker integration tests, leaving the individual branches (non-map input, missingisError, various rate-limit phrases, reset-time parsing) without dedicated coverage.Tests Added
TestExtractRateLimitErrorTextcontentkey returns default messageTestIsRateLimitToolResult(false, zero time)(false, zero time)isError: falsewith rate-limit text returns falseisErrorfield returns falseisError: truewith non-rate-limit text returns falseisError: truewith "rate limit exceeded" text returns trueisError: truewith "secondary rate limit" text returns trueisError: truewith "too many requests" text returns trueisError: truewith "rate limit 403" text returns trueisError: truewith reset-time in text returns true and non-zerotime.TimeisError: truewith empty content returns falseisError: truewith nocontentkey returns falseGenerated by Test Coverage Improver
Next run will target the next most complex under-tested function
Warning
Firewall blocked 8 domains
The following domains were blocked by the firewall during workflow execution:
awmgmcpggo.opentelemetry.iogo.yaml.ingolang.orggoogle.golang.orggopkg.inproxy.golang.orgreleaseassets.githubusercontent.comSee Network Configuration for more information.