chore: comprehensive coverage, correctness, and quality improvements#541
Merged
John-David Dalton (jdalton) merged 1 commit intomainfrom Apr 3, 2026
Merged
Conversation
Coverage: 70% → 96% lines (merged main + isolated suites) Source fixes: - Remove 138 lines of dead code from http-client.ts (unused retry wrappers) - Remove 62 unnecessary c8 ignore pragmas from socket-sdk-class.ts - Add #executeWithRetry to batchOrgPackageFetch, viewPatch, getApi, sendApi, downloadOrgFullScanFilesAsTar, streamFullScan, postOrgTelemetry - Fix postOrgTelemetry swallowing 5xx errors - Fix sendApi ResponseError handling (was returning status: 0) - Fix 6 malformed/orphaned JSDoc blocks - Remove dead type exports (CreateOrgFullScanOptions, CreateScanFromFilepathsOptions) Test improvements: - Add 6 new test files covering error paths, fail paths, streaming limits, http-client error branches, and coverage gaps (110+ new tests) - Remove 29 duplicate tests and 2 redundant test files - Fix cross-platform issues (hardcoded /tmp paths, timing-dependent tests) - Fix nock/forks incompatibility in coverage mode - Strengthen weak assertions (toBeDefined → specific values) Coverage infrastructure: - Fix cover.mjs aggregation: merge coverage-final.json via max-hit-count instead of naive sum-of-totals that double-counted shared source files - DRY cover.mjs: extract runTestSuites, mergeCoverageFinal, displayCodeCoverage (605 → ~400 lines, eliminated --code-only duplication) - Fix isolated test config: use singleFork: false for nock compatibility - Update coverage thresholds to 94% lines, 83% branches, 98% functions
John-David Dalton (jdalton)
force-pushed
the
chore/coverage-quality-improvements
branch
from
38c090e to
d275dca
Compare
Contributor
Author
|
Cursor (@cursor) review |
Dale Bustad (divmain)
approved these changes
Apr 3, 2026
John-David Dalton (jdalton)
deleted the
chore/coverage-quality-improvements
branch
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.
Summary
Coverage: 70% → 96% lines (merged main + isolated suites), 88% branches, 99% functions
Source fixes
http-client.ts(unused retry wrappers:withRetry,createDeleteRequestWithRetry,createGetRequestWithRetry,createRequestWithJsonAndRetry)c8 ignorepragmas fromsocket-sdk-class.ts— error catch blocks are now tested#executeWithRetryto 7 methods that were missing retry logic:batchOrgPackageFetch,viewPatch,getApi,sendApi,downloadOrgFullScanFilesAsTar,streamFullScan,postOrgTelemetrypostOrgTelemetryswallowing 5xx errors (was using#createQueryErrorResultinstead of#handleApiError)sendApiResponseError handling (was returningstatus: 0for HTTP errors)CreateOrgFullScanOptions,CreateScanFromFilepathsOptions)Test improvements
/tmppaths →os.tmpdir(), timing-dependentsetTimeout→ event-based)pool: 'forks')toBeDefined()→ specific value checks)Coverage infrastructure
cover.mjsaggregation: mergecoverage-final.jsonvia max-hit-count per statement/branch/function instead of naive sum-of-totals that double-counted shared source filescover.mjs: extractrunTestSuites(),mergeCoverageFinal(),displayCodeCoverage(),parseTypeCoveragePercent()— eliminated--code-onlyduplication (605 → ~400 lines)singleFork: falsefor nock compatibilityTest plan
pnpm run fix --allpassespnpm run check --allpassespnpm test— 605 tests, 23 files, all greenpnpm run cover— 96% lines merged, 97.8% cumulative, all thresholds met