Merged
Conversation
Covers the two core bucketing rules: major params (server_id, channel_id) get their own buckets, minor params (message_id) are normalized so requests against different messages in the same channel share a bucket. Also verifies that method is always part of the key and that url() produces the correct full URL from a given base.
Tests all six error types across the full inheritance chain. Each class gets checked for the right instanceof relationship, status code, and any extra fields specific to it (retryAfter/global/bucket on RateLimitError, optional code on HTTPError, etc).
Stubs global fetch within a scoped describe block so tests can verify what REST actually sends without making real network calls. Covers createServer (POST + JSON body), auth header format, Content-Type, listMessages (GET with query params), deleteChannel (DELETE, no body), and 204 No Content resolving to undefined.
Each HTTP status code now has a test verifying the right error class comes back. The tricky part was 429 and 5xx — the REST client retries those by default, so tests use maxRetries: 0 to make them throw on the first attempt rather than sleeping through retry delays and exhausting the fetch mock queue. RateLimitError gets extra coverage for the retryAfter, global flag, and bucket fields since those are the main reason that class exists.
…s [refs #6] The two existing tests only covered getServer. This expands coverage to getChannel, createMessage, getMessage (both IDs), and the before/after cursor params on listMessages. Also adds a positive case — a valid all-digit snowflake should pass validation and reach the network layer without throwing.
Member
Author
|
Worth noting the approach for anyone reviewing: fetch is stubbed inside a scoped describe block with describe('request basics', () => {
const fetchMock = vi.fn()
beforeEach(() => {
vi.stubGlobal('fetch', fetchMock)
fetchMock.mockReset()
})
afterEach(() => {
vi.unstubAllGlobals()
})
// ...
})The |
…s [refs #6] The token guard tests were making actual HTTP requests to api.intent.chat. Locally they'd fail fast on connection refused, but in CI there's no network so they'd hang until Vitest's 5s test timeout fired. Stubbed fetch with an immediate TypeError in those tests — the token guard check throws IntentError before fetch is even reached, and once a token is set the TypeError surfaces correctly as a non-IntentError.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #6
The REST client had no real test coverage. This adds four new test files and expands the existing one to cover everything the issue asked for.
Route bucket keys
Route.test.tsis new. Verifies that major params (server ID, channel ID) produce separate rate limit buckets while minor params (message ID) get normalized so requests against different messages in the same channel share one.Error class hierarchy
errors.test.tsis new. Every error class gets tested for the right inheritance chain and the fields specific to it.RateLimitErrorgets extra attention sinceretryAfter,global, andbucketare the whole point of that class existing.Request basics
REST.test.tsnow stubsglobal.fetchinside a scoped describe block to verify that REST actually sends the right method, URL, auth header, and body. The stub is cleaned up after each test so it doesn't affect the existing token guard tests.Error status mapping
429, 401, 403, 404, 500, 502, and unknown 4xx each get a test asserting the right error type comes back. The 429 and 5xx tests use
maxRetries: 0so they throw immediately rather than sleeping through retry delays and exhausting the fetch mock queue.Snowflake validation
The original tests only covered
getServer. This expands togetChannel,createMessage,getMessage(both IDs separately), and thebefore/aftercursors onlistMessages. Also adds a positive case confirming a valid numeric snowflake passes through to the network layer.