Cover every pm-slack code path with behavioural tests and raise the coverage gate to 100 - #112
Merged
Merged
Conversation
…to 100 - Add test/coverage.test.ts with 127 behavioural tests driving every line, branch, and function in index.ts to 100% coverage - Remove dead beforeCommand fallback hook (SDK always provides afterCommand) - Remove dead postToSlack throw ternary (postToSlackOnce only rejects Error) - Remove dead statusCode ?? 0 (HTTP responses always have status codes) - Simplify statusIsClosed parameter to string (callers always pass strings) - Remove dead ALL_EVENTS.find ?? 'create' (parseEvents always non-empty) - Introduce toErrorMessage() helper consolidating 4 catch-block ternaries - Raise coverageGate thresholds from 77/85/79 to 100/100/100
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reviewer's GuideThe PR adds a large behavioral test suite that drives pm-slack through real SDK command, hook, preflight, filesystem, and local HTTP paths, removes unreachable fallback code, centralizes error formatting, updates generated distribution files, and raises coverage enforcement to 100% for lines, branches, and functions. Sequence diagram for pm-slack Slack notification behaviorsequenceDiagram
participant Test as BehavioralTest
participant SDK as PMSDKHarness
participant Handler as CommandHandler
participant Slack as LocalHTTPServer
Test->>SDK: run command
SDK->>Handler: invoke handler
Handler->>Slack: POST webhook payload
Slack-->>Handler: HTTP response
Handler-->>SDK: command result
SDK-->>Test: assert behavior
Flow diagram for pm-slack behavioral coverage pathsflowchart LR
Tests[Behavioral test suite] --> SDK[PM SDK test harness]
Tests --> Hooks[runHook lifecycle tests]
Tests --> Preflight[Direct preflight run calls]
Tests --> HTTP[Local HTTP server tests]
SDK --> Handlers[Command handlers]
Hooks --> AfterCommand[afterCommand hook]
Preflight --> Gate[Webhook validation gate]
HTTP --> Slack[Slack webhook paths]
Handlers --> Coverage[100 percent coverage gate]
AfterCommand --> Coverage
Gate --> Coverage
Slack --> Coverage
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…ttps test - Remove unnecessary 100ms sleeps (runHook awaits the hook which awaits postToSlack) - Use dynamically allocated ports instead of hardcoded port 1 - Use https://hooks.slack.com/services/A/B/C for config-only tests (no network call) - Add https://localhost test to cover https.request and port 443 default branches - Fix remaining withEnv async callback to use withEnvAsync
The coverage raise replaced 'ALL_EVENTS.find(...) ?? "create"' with 'as EventKind' at both command sites. The fallback was unreachable through parseEvents, which never returns an empty set, but the cast moved that invariant out of the compiler's sight. The next change to parseEvents could then have produced an undefined header verb silently. primaryEvent(events) restores the original behaviour (first selected event in ALL_EVENTS priority order, create when nothing is selected) as a small pure function. Its empty-set contract is unit-tested directly, so the branch is real and covered, not asserted away. The remaining two narrowings (a ClientRequest response always has statusCode; postToSlack's lastErr is always an Error) now state the runtime invariant they rely on.
![hound[bot]](https://avatars.githubusercontent.com/in/3598?s=60&v=4){kind=small}
…holds port 443 The test drove https://localhost/hook to cover the default-port and https.request branches, and relied on nothing listening on local port 443. A proxy or dev server there would change the outcome or hit the 10-second timeout. The host now uses the RFC 6761 .invalid TLD, which can never resolve, so the transport error is deterministic on every machine.
Owner
Author
|
Orchestrator review of this PR, applied in e438506 and 645d780:
Local: 255/255 tests, 100/100/100 coverage, docstrings 53/53, changelog up to date. @greptileai please review the new commits. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- repository status ---'
git status --short
printf '%s\n' '--- recent commits ---'
git log --oneline -8
printf '%s\n' '--- merge base and diff stat ---'
base="$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main)"
printf 'base=%s\n' "$base"
git diff --stat "$base"...HEAD
printf '%s\n' '--- changed files ---'
git diff --name-status "$base"...HEAD
printf '%s\n' '--- source outlines ---'
fd -t f -e ts -e json -e md . | sort | sed -n '1,240p'
printf '%s\n' '--- TypeScript outlines ---'
fd -t f -e ts . -x ast-grep outline {} --items all 2>/dev/null | sed -n '1,360p'
printf '%s\n' '--- package and manifest paths ---'
fd -t f -a 'package.json' -a 'manifest.json' -a 'CHANGELOG.md' . | sortLength of output: 22291 🏁 Script executed: #!/bin/bash
set -euo pipefail
base="$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main)"
printf '%s\n' '--- index production diff ---'
git diff --unified=80 "$base"...HEAD -- index.ts
printf '%s\n' '--- package / manifest / generated declaration diff ---'
git diff --unified=40 "$base"...HEAD -- package.json manifest.json dist/index.d.ts CHANGELOG.md
printf '%s\n' '--- generated runtime diff ---'
git diff --unified=25 "$base"...HEAD -- dist/index.js
printf '%s\n' '--- coverage test start and helpers ---'
sed -n '1,260p' test/coverage.test.ts
printf '%s\n' '--- all local HTTP server construction and close calls ---'
rg -n -C 8P '\b(createServer|listen|close)\s*\(' test/coverage.test.ts
printf '%s\n' '--- direct use of exported test helpers ---'
rg -n -C 3 '__test__\.' test/coverage.test.ts
printf '%s\n' '--- manifest compatibility test ---'
sed -n '1,120p' test/compatibility-floor.test.ts
printf '%s\n' '--- relevant manifest ---'
cat manifest.jsonLength of output: 125354 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- coverage test index ---'
rg -n '^test\(' test/coverage.test.ts
printf '%s\n' '--- network tests and server lifecycle ---'
rg -n -C 8 -P '\b(createServer|listen|close)\s*\(' test/coverage.test.ts
printf '%s\n' '--- test global mutation sites ---'
rg -n -C 5 -P '(process\.env|console\.(error|log)|process\.stdout\.(write)|process\.stderr\.(write))' test/coverage.test.ts
printf '%s\n' '--- primary event and error helper tests ---'
rg -n -C 12 -P '(primaryEvent|toErrorMessage)' test/coverage.test.ts
printf '%s\n' '--- status code and retry invariants test context ---'
rg -n -C 12 -P '(statusCode|postToSlackOnce|ENOTFOUND|\.invalid|lastErr)' test/coverage.test.ts
printf '%s\n' '--- coverage gate implementation ---'
sed -n '1,380p' scripts/coverage-gate.ts
printf '%s\n' '--- manifest and compatibility floor test ---'
cat manifest.json
sed -n '1,150p' test/compatibility-floor.test.ts
printf '%s\n' '--- artifact export tails ---'
tail -n 90 index.ts
tail -n 90 dist/index.js
printf '%s\n' '--- package lock diff / package scripts ---'
git diff --name-status "$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main)"...HEAD -- package-lock.json
node -e 'const p=require("./package.json"); console.log(JSON.stringify({scripts:p.scripts,devDependencies:p.devDependencies,coverageGate:p.coverageGate},null,2))'Length of output: 89538 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- coverage test working-directory and temp-root contexts ---'
sed -n '450,530p' test/coverage.test.ts
sed -n '1160,1278p' test/coverage.test.ts
sed -n '1518,1648p' test/coverage.test.ts
sed -n '1970,2130p' test/coverage.test.ts
sed -n '2280,2438p' test/coverage.test.ts
printf '%s\n' '--- working directory mutation searches across tests ---'
rg -n -C 6 -P '(process\.chdir|cwd\(|mkdtempSync|rmSync)' test
printf '%s\n' '--- test runner configuration ---'
cat tsconfig.test.json
cat tsconfig.json
printf '%s\n' '--- workflows and coverage invocations ---'
fd -t f -p '.github' -e yml -e yaml -x sh -c 'echo "--- $1"; rg -n -C 4 -P "(npm run (test|coverage|release:check)|node --test|coverage)" "$1"' _ {}
printf '%s\n' '--- `.invalid` test exact history ---'
git blame -L 2410,2427 test/coverage.test.ts
printf '%s\n' '--- changed source type-check-sensitive declarations ---'
sed -n '1230,1412p' index.tsLength of output: 62177
The remaining reviewed changes are consistent. The You are interacting with an AI system. |
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
1 participant
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.
Summary
Brings
pm-slackto a real 100% line / branch / function coverage and raises the coverage gate from 77/85/79 to 100/100/100.Before / After
What changed
test/coverage.test.tsdriving every uncovered line, branch, and function through real behaviour: command handlers via the pm SDK test harness, lifecycle hooks viarunHook, preflight override via directruncalls, and HTTP paths via localnode:httpservers on 127.0.0.1.beforeCommandfallback hook — the SDK'sExtensionApiRegistraralways providesapi.hooks.afterCommand, so theelse ifbranch is unreachable.postToSlackthrow ternary —postToSlackOnceonly rejects withErrorinstances, solastErr instanceof Erroris always true.res.statusCode ?? 0— HTTP responses always carry a status code.statusIsClosed?? ""—aggregateDigestalways passes a lowercased string.ALL_EVENTS.find(...) ?? "create"(2 sites) —parseEventsalways returns a non-empty set of validEventKindvalues.toErrorMessage()helper consolidates 4 identicalerr instanceof Error ? err.message : String(err)ternaries from catch blocks into one function, exported via__test__with both arms tested.package.json.Resolves
pm-slack-hn3n
Summary by Sourcery
Achieve complete behavioral coverage for pm-slack and enforce 100% line, branch, and function coverage.
Enhancements:
Build:
Tests:
Chores:
Summary by cubic
Raises
pm-slackto 100% line, branch, and function coverage and closespm-slack-hn3n. Runtime behavior is unchanged; the coverage gate inpackage.jsonmoves from 77/85/79 to 100/100/100.Tests
@unbrained/pm-cli/sdktest harness and localnode:httpservers instead of mocks; the HTTPS default-port case fails deterministically at name resolution on an unresolvable.invalidhost.Refactors
beforeCommandfallback hook andstatusCode ?? 0.toErrorMessage(), exported via__test__.ALL_EVENTS.find(...) ?? "create"fallback with aprimaryEvent()helper so the create-when-empty branch stays real and covered.Written for commit 645d780. Summary will update on new commits.