Conversation
Signed-off-by: Brian DeHamer <bdehamer@github.com>
There was a problem hiding this comment.
Pull request overview
This pull request refactors the test suite to improve modularity, maintainability, and organization. The main changes include reorganizing tests into unit and integration directories, creating a centralized mock factory for reusable test utilities, adding sample fixture files for testing, and improving test coverage by adding Istanbul ignore comments to production code for hard-to-test branches.
Changes:
- Reorganized tests from flat
__tests__/structure into__tests__/unit/and__tests__/integration/directories - Introduced centralized mock factory at
__tests__/fixtures/mocks.tswith reusable mock creation functions - Enhanced existing unit tests with more comprehensive test cases and improved test descriptions
- Added Istanbul ignore comments to production code for defensive code paths that are difficult to test
- Created sample fixture files (SPDX SBOM, CycloneDX SBOM, predicate) for testing
- Removed
"peer": truemarkers from package-lock.json (normal npm update)
Reviewed changes
Copilot reviewed 21 out of 23 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
__tests__/unit/subject.test.ts |
Moved from root, enhanced with comprehensive test cases for subject input validation, digest formats, glob patterns, and checksums |
__tests__/unit/style.test.ts |
Moved from root with improved test descriptions |
__tests__/unit/sbom.test.ts |
Moved from root, reorganized test structure with better grouping |
__tests__/unit/predicate.test.ts |
Moved from root with improved test organization |
__tests__/unit/detect.test.ts |
Reorganized from nested describe blocks to flatter structure with clearer test names |
__tests__/integration/main.test.ts |
New comprehensive integration test for main workflow with full mocking |
__tests__/integration/attest.test.ts |
New integration test for attestation creation and storage record functionality |
__tests__/integration/provenance.test.ts |
Moved to integration directory, uses shared mock fixtures |
__tests__/index.test.ts |
Enhanced to verify actual input mapping rather than just checking if run was called |
__tests__/fixtures/mocks.ts |
New mock factory with reusable creation functions for core, GitHub, attest, and OCI mocks |
__tests__/fixtures/samples/*.json |
New sample fixture files for testing SBOM and predicate parsing |
src/main.ts |
Added Istanbul ignore comments for hard-to-test code paths (optional fields, environment checks) |
src/attest.ts |
Added Istanbul ignore comment for defensive protocol validation |
dist/index.js |
Compiled output reflecting source changes |
package-lock.json |
Removed "peer": true markers from dependencies (normal npm update) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| rest: { | ||
| repos: { | ||
| get: jest | ||
| .fn<RestEndpointMethodTypes['repos']['get']['response']>() |
There was a problem hiding this comment.
The generic parameter for jest.fn should be a function signature, not a response type. Currently, it's typed as jest.fn<RestEndpointMethodTypes['repos']['get']['response']>(), but it should be jest.fn<() => Promise<RestEndpointMethodTypes['repos']['get']['response']>>() to match the function signature pattern used elsewhere in the codebase.
| .fn<RestEndpointMethodTypes['repos']['get']['response']>() | |
| .fn<() => Promise<RestEndpointMethodTypes['repos']['get']['response']>>() |
This pull request refactors and expands the test suite for the attestation functionality, introducing improved test modularity, mock factories, and new sample fixtures. The main changes include moving and enhancing integration tests, consolidating and generalizing mock creation, and adding sample data for more comprehensive testing.