Add more tests#208
Open
connorads wants to merge 6 commits into
Open
Conversation
Phase 0 safety net before the dependency migrations. The Slack handler had zero coverage; these characterise the exact seam every Bolt / serverless-express / Express upgrade will touch, so later phases can prove they're behaviour preserving: - handle-command: acks before responding, maps destination -> response_type (channel -> in_channel, user -> ephemeral), the ephemeral error fallback, and redaction of token/trigger_id/response_url from logs - command-parsers: parseUnlock force rule + empty input, getFirstParam In-memory fakes (no mocks) and no DynamoDB, so they run under plain jest. Verified: jest 14/14, tsc --noEmit, eslint clean.
Extract the inline installationStore from infra.ts into a testable factory and pin its behaviour against DynamoDB Local. The OAuth install path was the single biggest untested surface and is exactly what the Bolt 3->4 and AWS SDK v2->v3 migrations touch. - src/handlers/slack/installation-store.ts: factory, behaviour identical to the old inline store; infra.ts now delegates to it - test/utils.ts: recreateInstallationsTable (the installations table was never created by the test harness) - test/installation-store.test.ts: store/fetch round-trip, enterprise store + fetch rejection, and the current throw-on-unknown-team behaviour (pinned so the SDK v3 migration must consciously change it) Verified vs DynamoDB Local (Docker): 18/18 new + 64/64 existing suites, tsc --noEmit, eslint. Saw the enterprise assertion go red before green.
Extract the swagger Express app from index.ts into an injectable factory and supertest it in-process. index.ts keeps requiring swagger.html / openapi.json (webpack text-loader) and reading SERVERLESS_STAGE, then passes them in, so the test needs no html transform, no env and no serverless-offline. Pins: HTML at /api-docs, the OpenAPI doc at both /openapi.json and /api-docs/openapi.json, and the stage-aware servers field. The stage is now applied to a copy, so the handler no longer mutates the required openapi.json module (asserted). Verified: swagger 5/5 in-process; 87/87 runnable suites green vs DynamoDB Local; tsc --noEmit; eslint.
The phase-0 characterisation commits left comments framed around the act
of change ("extracted verbatim", "now applied to a copy rather than") and
around unstarted future work ("the AWS SDK v3 migration", "every Bolt /
serverless-express migration touches"). Both go stale: diff-relative
language is meaningless once the old code is gone, and forward references
mislead once the migration lands.
That context — these characterisation tests are a safety net before the
Bolt / serverless-express / AWS SDK v3 migrations — belongs here in the
history, not in source that outlives it.
Kept only the durable nuggets in-code:
- installation-store.test.ts: why an unknown team throws (accidental
destructure of undefined) — explains a non-obvious assertion.
- handle-command.test.ts: what the setup helper records.
Dropped the swagger/app.ts copy-not-mutate note (already pinned by a test)
and the "extracted from" headers entirely.
GitHub retired the ubuntu-20.04 runner image, so build, deploy and CodeQL jobs fail to start. Move all four to ubuntu-latest so the safety-net test suite can actually run on PRs.
DynamoDB Local now validates credentials and rejects the empty '' values with UnrecognizedClientException, failing every dynamodb repo test. Older images ignored creds; the latest image pulled by rrainn/dynamodb-action no longer does. Use non-empty placeholders.
|
|
coverage: 93.75% (-5.7%) from 99.401% — test/phase0-safety-net into master |
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
2 participants
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.
No description provided.