fix(test): Stabilize test coverage by preventing jiti double instrumentation#928
fix(test): Stabilize test coverage by preventing jiti double instrumentation#928
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR introduces dependency injection for config file loading via a pluggable jitiImport function, refactors test fixtures to export plain objects instead of defineConfig-wrapped configs, and adds comprehensive test coverage for CLI actions, spinner behavior, permission checking, and output generation features. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as loadFileConfig Caller
participant LFC as loadFileConfig
participant LAVC as loadAndValidateConfig
participant DepsImporter as deps.jitiImport
Caller->>LFC: loadFileConfig(rootDir, argConfigPath, deps?)
alt deps provided
LFC->>LAVC: loadAndValidateConfig(filePath, deps)
else deps not provided
LFC->>LFC: create defaultJitiImport
LFC->>LAVC: loadAndValidateConfig(filePath, {jitiImport: defaultJitiImport})
end
alt TS or JS config file
LAVC->>DepsImporter: deps.jitiImport(filePath)
DepsImporter->>LAVC: return loaded config
else JSON or json5 config
LAVC->>LAVC: parse JSON/json5 directly
end
LAVC->>LFC: return validated config
LFC->>Caller: return config
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
Summary of ChangesHello @yamadashy, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the stability and performance of the test suite by addressing a critical issue related to jiti's dynamic module loading. Previously, test coverage was inconsistent due to jiti inadvertently re-instrumenting source files already handled by Vitest. The solution involves introducing a dependency injection pattern for jiti imports, simplifying test fixtures, and explicitly mocking jiti in integration tests, resulting in stable and higher coverage, along with a substantial reduction in test execution time. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #928 +/- ##
===========================================
+ Coverage 75.32% 89.50% +14.18%
===========================================
Files 111 111
Lines 7793 7807 +14
Branches 1454 1498 +44
===========================================
+ Hits 5870 6988 +1118
+ Misses 1923 819 -1104 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Deploying repomix with
|
| Latest commit: |
3811fdb
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://81e4a9b0.repomix.pages.dev |
| Branch Preview URL: | https://chore-coverage.repomix.pages.dev |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a critical issue of test coverage instability by preventing double instrumentation of files during testing. The changes include adding a DI pattern to loadFileConfig, removing defineConfig imports from test fixtures, and mocking jiti in dynamic config tests. These modifications stabilize test coverage, improve test runtime, and enhance the accuracy of instrumentation. The code changes are well-structured and effectively resolve the identified root cause.
PR Review: Stabilize Test Coverage by Preventing Jiti Double InstrumentationSummaryExcellent work on identifying and fixing this coverage instability issue! The root cause analysis is thorough, and the solution follows best practices with dependency injection. The PR successfully resolves the double instrumentation problem and dramatically improves both coverage stability and test performance. 🎯 Strengths
💡 Code Quality Assessmentsrc/config/configLoad.tsPositive:
Minor Observations:
Details: Implementation PatternThe DI pattern implementation is exemplary: export const loadFileConfig = async (
rootDir: string,
argConfigPath: string | null,
deps = {
jitiImport: defaultJitiImport,
},
): Promise<RepomixConfigFile> => {This allows tests to inject mock implementations while production code uses the real jiti import, avoiding coverage contamination. Test ChangesPositive:
Observations:
🔒 Security Considerations✅ No security concerns identified
⚡ Performance Considerations✅ Significant performance improvements
🧪 Test Coverage✅ Excellent test coverage improvements New tests added for:
The coverage increase from ~68-71% to 89.34% represents genuine improvement, not just stabilization. 🔍 Premortem Analysis: Potential Failure Scenarios1. Edge Case: User-Provided Config Files in ProductionRisk: Low
2. Integration Issue: Third-Party Test RunnersRisk: Low
3. Deployment Risk: Coverage Reporting in CIRisk: Very Low
4. Regression Risk: Mock DriftRisk: Low
5. Breaking Change: Internal APIRisk: Very Low
📋 Suggested Improvements (Optional)1. Type Safety EnhancementConsider adding a type definition for the // Type for dependency injection
type ConfigLoadDeps = {
jitiImport: (fileUrl: string) => Promise<unknown>;
};
export const loadFileConfig = async (
rootDir: string,
argConfigPath: string | null,
deps: ConfigLoadDeps = {
jitiImport: defaultJitiImport,
},
): Promise<RepomixConfigFile> => {This makes the contract more explicit and helps with IDE autocomplete. 2. Test Documentation EnhancementThe dynamic config tests have excellent comments, but consider adding a comment in the main test file explaining why only dynamic tests are mocked: // Static config tests use real jiti to verify actual loading behavior
// Dynamic config tests mock jiti to prevent coverage instability3. Consider E2E Test for Dynamic ConfigsAdd a separate E2E test file that verifies dynamic config loading works correctly without affecting coverage: // tests/e2e/dynamic-config.e2e.test.ts
// Runs in isolation to verify real dynamic config behaviorThis would provide confidence that mocked tests accurately reflect real behavior. ✅ Checklist Review
🎉 RecommendationAPPROVE - This PR is ready to merge! The implementation is clean, well-tested, and solves a real problem that was causing coverage instability. The 38x performance improvement on integration tests is a significant bonus. The premortem analysis identified only low-risk scenarios with appropriate mitigations already in place. The only suggestion is to update CI coverage thresholds to reflect the new 89.34% baseline to prevent false positives in future PRs. Great work on the thorough investigation and elegant solution! 🚀 |
There was a problem hiding this comment.
Pull Request Overview
This PR improves test stability and code coverage by addressing issues with dynamic imports and test fixtures. The changes focus on preventing jiti from transforming source files during tests and making tests more deterministic.
- Removed
defineConfigimports from test fixture files to prevent jiti from transformingsrc/files during test execution - Made dynamic timestamp values in test fixtures deterministic to ensure stable coverage
- Added comprehensive unit tests for output generation, permission checking, config loading, CLI spinner, CLI reporting, and init actions
- Introduced dependency injection for
jitiImportin the config loading logic to enable better testability
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/fixtures/config-ts/repomix.config.ts | Removed defineConfig import and wrapper to prevent jiti transformation during tests |
| tests/fixtures/config-ts/repomix.config.mts | Removed defineConfig import and wrapper to prevent jiti transformation during tests |
| tests/fixtures/config-ts/repomix.config.cts | Removed defineConfig import and wrapper to prevent jiti transformation during tests |
| tests/fixtures/config-ts/repomix-dynamic.config.ts | Replaced dynamic timestamp with fixed value and removed defineConfig import |
| tests/fixtures/config-js/repomix.config.mjs | Removed defineConfig import and wrapper to prevent jiti transformation during tests |
| tests/fixtures/config-js/repomix.config.js | Removed defineConfig import and wrapper to prevent jiti transformation during tests |
| tests/fixtures/config-js/repomix-dynamic.config.js | Replaced dynamic timestamp with fixed value and removed defineConfig import |
| tests/core/output/outputGenerate.test.ts | Added new test cases for git diffs, git logs, JSON output, and conditional output sections |
| tests/core/file/permissionCheck.test.ts | Added edge case tests for macOS-specific error handling and unknown error codes |
| tests/config/configLoad.test.ts | Added error handling tests for unsupported formats and general errors |
| tests/config/configLoad.integration.test.ts | Added mock jiti implementation to prevent coverage instability in dynamic config tests |
| tests/cli/cliSpinner.test.ts | Created comprehensive test suite for the CLI spinner functionality |
| tests/cli/cliReport.test.ts | Added tests for git diffs/logs reporting and security check details |
| tests/cli/actions/initAction.test.ts | Added tests for user cancellation scenarios and successful creation paths |
| src/config/configLoad.ts | Added dependency injection for jitiImport function to enable testability |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/config/configLoad.ts(5 hunks)tests/cli/actions/initAction.test.ts(2 hunks)tests/cli/cliReport.test.ts(2 hunks)tests/cli/cliSpinner.test.ts(1 hunks)tests/config/configLoad.integration.test.ts(2 hunks)tests/config/configLoad.test.ts(1 hunks)tests/core/file/permissionCheck.test.ts(1 hunks)tests/core/output/outputGenerate.test.ts(1 hunks)tests/fixtures/config-js/repomix-dynamic.config.js(1 hunks)tests/fixtures/config-js/repomix.config.js(2 hunks)tests/fixtures/config-js/repomix.config.mjs(1 hunks)tests/fixtures/config-ts/repomix-dynamic.config.ts(1 hunks)tests/fixtures/config-ts/repomix.config.cts(1 hunks)tests/fixtures/config-ts/repomix.config.mts(1 hunks)tests/fixtures/config-ts/repomix.config.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
tests/cli/cliReport.test.ts (4)
tests/testing/testUtils.ts (1)
createMockConfig(15-45)src/core/packager.ts (1)
PackResult(18-32)src/cli/cliReport.ts (2)
reportSummary(49-94)reportSecurityCheck(96-129)src/core/security/securityCheck.ts (1)
SuspiciousFileResult(10-14)
tests/cli/actions/initAction.test.ts (1)
src/cli/actions/initAction.ts (2)
createConfigFile(41-126)createIgnoreFile(128-178)
tests/config/configLoad.test.ts (1)
src/config/configLoad.ts (1)
loadFileConfig(69-111)
tests/core/output/outputGenerate.test.ts (2)
tests/testing/testUtils.ts (1)
createMockConfig(15-45)src/core/output/outputGenerate.ts (1)
generateOutput(221-262)
src/config/configLoad.ts (2)
src/index.ts (2)
loadFileConfig(31-31)RepomixConfigFile(32-32)src/config/configSchema.ts (1)
RepomixConfigFile(153-153)
tests/fixtures/config-ts/repomix-dynamic.config.ts (1)
tests/fixtures/config-js/repomix-dynamic.config.js (1)
timestamp(5-5)
tests/cli/cliSpinner.test.ts (1)
src/cli/cliSpinner.ts (1)
Spinner(9-70)
tests/config/configLoad.integration.test.ts (1)
src/config/configLoad.ts (1)
loadFileConfig(69-111)
tests/core/file/permissionCheck.test.ts (1)
src/core/file/permissionCheck.ts (2)
checkDirectoryPermissions(27-90)PermissionError(16-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Agent
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Test (macos-latest, 22.x)
- GitHub Check: Test with Bun (windows-latest, latest)
- GitHub Check: Test (macos-latest, 20.x)
- GitHub Check: Test (windows-latest, 24.x)
- GitHub Check: Build and run (windows-latest, 24.x)
- GitHub Check: Build and run with Bun (windows-latest, latest)
- GitHub Check: claude-review
🔇 Additional comments (18)
src/config/configLoad.ts (3)
60-67: LGTM! Clean dependency injection implementation.The
defaultJitiImporthelper provides a sensible default withmoduleCache: falsefor fresh config loads andinteropDefault: truefor automatic default export handling. This enables testability while maintaining production behavior.
69-111: LGTM! Well-designed dependency injection pattern.The optional
depsparameter with a default value maintains backward compatibility while enabling test mocking. All code paths (argConfigPath, local config, global config) consistently thread the dependency through toloadAndValidateConfig.
118-165: LGTM! Consistent DI implementation in config loading.The
loadAndValidateConfigfunction now usesdeps.jitiImportinstead of directly instantiating jiti, enabling testability. The JSON/JSON5 paths remain unchanged, ensuring no behavioral changes for non-JS/TS configs.tests/config/configLoad.test.ts (1)
210-234: LGTM! Comprehensive error handling test coverage.The new test cases thoroughly validate error scenarios including unsupported file formats, permission errors, and non-Error objects. This ensures robust error handling across different failure modes.
tests/cli/cliReport.test.ts (2)
57-165: LGTM! Thorough test coverage for Git-related reporting.The new test cases comprehensively validate Git diffs and logs reporting in both scenarios: when data is present (with token counts) and when absent. This ensures the reporting logic handles all cases correctly.
239-295: LGTM! Excellent coverage for security issue reporting edge cases.The new tests validate correct pluralization ("security issue" vs "security issues") and comprehensive Git diff/log security reporting. This ensures accurate user-facing messages across different scenarios.
tests/fixtures/config-ts/repomix-dynamic.config.ts (1)
1-16: LGTM! Excellent refactoring to prevent double instrumentation.The comments clearly explain the rationale, and the fixed timestamp ensures deterministic test coverage. This change directly addresses the root cause of coverage instability mentioned in the PR objectives.
tests/fixtures/config-ts/repomix.config.mts (1)
1-11: LGTM! Consistent refactoring with clear documentation.The plain object export avoids jiti transformation of src/ files during tests, preventing the double instrumentation issue. The comments ensure future maintainers understand the rationale.
tests/fixtures/config-ts/repomix.config.cts (1)
1-11: LGTM! Fixture refactoring aligns with PR objectives.The transition from
defineConfigwrapper to plain object export, along with explanatory comments, successfully prevents src/ file transformation during test runs.tests/fixtures/config-js/repomix-dynamic.config.js (1)
1-15: LGTM! Clean refactoring with fixed timestamp for determinism.The fixed timestamp constant and plain object export ensure stable, deterministic test coverage. The comments effectively communicate the intent to prevent double instrumentation.
tests/fixtures/config-ts/repomix.config.ts (1)
1-12: LGTM! Consistent pattern applied across all fixtures.The plain object export and explanatory comments align with the overall PR strategy to stabilize test coverage by preventing jiti from transforming src/ files during test fixture loading.
tests/core/output/outputGenerate.test.ts (5)
201-220: LGTM! Git diffs integration test is well-structured.The test correctly validates that git diffs are included in the output when
config.output.git.includeDiffsis enabled, checking for both work tree and staged diff content.
222-254: LGTM! Git logs integration test covers the key scenarios.The test appropriately validates that git logs are included when
config.output.git.includeLogsis enabled, checking for the logs section header and commit details.
256-276: LGTM! JSON output test validates the format correctly.The test appropriately validates the JSON output style, ensuring the files are correctly mapped as a JSON object with file paths as keys and content as values.
278-292: LGTM! File exclusion test validates the negative case correctly.The test appropriately verifies that when
config.output.filesis set to false, file entries are excluded from the generated output.
294-307: LGTM! Directory structure exclusion test validates the feature correctly.The test appropriately verifies that when
config.output.directoryStructureis set to false, the directory structure section is excluded from the generated output.tests/config/configLoad.integration.test.ts (2)
53-78: LGTM! This is the key fix for coverage instability.The jitiImport mock prevents jiti from transforming src/ files during test execution, which was causing double instrumentation. The test still validates config structure and the loading mechanism while ensuring deterministic results.
Note: The trade-off is that this test no longer validates the actual fixture file content, but this is intentional and documented in the comments.
125-150: LGTM! JavaScript config test mirrors the TypeScript approach.The jitiImport mock consistently prevents coverage instability across both JavaScript and TypeScript config loading scenarios. The implementation follows the same pattern as the TypeScript test while using an appropriate mock fixture for JavaScript files.
Added comprehensive test coverage for critical CLI and core functionality: - Created new test file for cliSpinner with 15 tests covering: * Spinner start/stop/update operations * Quiet/verbose/stdout mode handling * Success/fail message display * Interval management - Enhanced initAction tests (11→17 tests): * Added isCancel handling for user cancellation * Added return value validation tests * Covered config and ignore file creation flows - Enhanced cliReport tests (8→15 tests): * Added git diffs/logs reporting tests * Added security check reporting for git content * Added single vs multiple issue message handling - Enhanced permissionCheck tests (13→16 tests): * Added macOS-specific error message tests * Added platform-specific error handling tests * Added unknown error code handling - Enhanced outputGenerate tests (7→12 tests): * Added git diffs/logs inclusion tests * Added JSON format output tests * Added file/directory structure exclusion tests Overall improvements: - Test count: 804 → 840 (+36 tests) - Code coverage: 70.63% → 71.00% (+0.37%) - Branch coverage: 77.64% → 78.55% (+0.91%) - Significant improvement in CLI modules (cliSpinner: 25% → 59.61%)
Added comprehensive error handling tests for configLoad.ts to improve coverage: - Added test for unsupported config file format (.yaml, .toml) - Added test for general error handling (permission denied) - Added test for non-Error object error handling - Improved branch coverage for error cases in loadAndValidateConfig This addresses the coverage drop that occurred in commit 156897d when TypeScript/JavaScript config file support was added. Improvements: - Test count: 840 → 844 (+4 tests) - Config branch coverage: 80.88% → 86.11% (+5.23%) - Overall branch coverage: 78.55% → 78.76% (+0.21%)
…ntation This commit resolves test coverage instability that occurred after implementing JS/TS config support. Coverage was fluctuating between 68-71% on each test run, caused by jiti dynamically transforming src/ files that were already instrumented by Vitest for coverage tracking. Root Cause: - Test fixture files imported `defineConfig` from src/index.js - jiti transformed the entire src/ directory when loading fixtures - Same files existed in two versions (Vitest-instrumented vs jiti-transformed) - v8 coverage tracker produced non-deterministic results Solution: 1. Added DI pattern to loadFileConfig with defaultJitiImport factory 2. Removed defineConfig imports from all test fixtures (use plain objects) 3. Mock jiti in dynamic config tests to prevent src/ transformation Results: - Coverage now stable at 89.34% (previously 68-71%) - Coverage improved by ~19% due to accurate instrumentation - Integration test runtime improved from 3.8s to 0.1s - All tests pass with zero lint errors Related Changes: - Reverted temporary Vitest config workarounds (sequence.shuffle, isolate) - Extracted defaultJitiImport as reusable factory function
Implemented mandatory and recommended fixes from code review: 1. Fix timer cleanup in cliSpinner.test.ts - Added vi.useRealTimers() to afterEach hook - Prevents fake timers from bleeding into other test files 2. Fix isCancel mock in initAction.test.ts (3 locations) - Changed from mockReturnValue(true) to mockImplementation - Now properly tests only the actual cancel symbol - More accurate test behavior for cancellation scenarios 3. Add DI pattern explanation comment in configLoad.ts - Clarifies why dependency injection is used for jitiImport - Documents the double instrumentation issue - Helps future maintainers understand the design decision
yamadashy
force-pushed
the
chore/coverage
branch
from
19c2706 to
3811fdb
Compare
Pull Request ReviewExcellent work on resolving the test coverage instability issue! This PR demonstrates a deep understanding of the problem and implements a clean solution. The coverage improvement from ~68-71% (unstable) to 89.34% (stable) is impressive. Summary✅ Approved - This PR successfully resolves the double instrumentation issue through dependency injection while maintaining code quality and improving test coverage. Strengths
Code Quality AssessmentArchitecture (Excellent)
Testing (Excellent)
Potential Issues & SuggestionsDetails1. Type Safety Consideration (Minor)Location: The // Current
const defaultJitiImport = async (fileUrl: string): Promise<unknown> => {
// ...
};
// Suggestion (if beneficial)
type JitiImportFn = (fileUrl: string) => Promise<unknown>;This would make the dependency injection interface more explicit, though the current approach is perfectly acceptable. 2. Test Mock VerificationLocation: The mock implementation verifies the file URL contains the expected filename: expect(fileUrl).toContain('repomix-dynamic.config.ts');Consider using a more precise check to ensure the full path is correct: expect(fileUrl).toMatch(/repomix-dynamic\.config\.ts$/);3. Error Handling Edge CaseLocation: The error handling is comprehensive, but consider logging the original error for debugging: } catch (error) {
rethrowValidationErrorIfZodError(error, 'Invalid config schema');
if (error instanceof SyntaxError) {
logger.trace('Syntax error details:', error); // Add trace logging
throw new RepomixError(`Invalid syntax in config file ${filePath}: ${error.message}`);
}
// ...
}Premortem AnalysisPotential Failure Scenarios & MitigationsScenario 1: Real-world Dynamic Configs FailRisk: Users with complex dynamic configs might encounter issues if the mock doesn't capture all use cases
Scenario 2: Jiti API ChangesRisk: Future jiti updates might change the import behavior
Scenario 3: Coverage RegressionRisk: Future changes might reintroduce double instrumentation
Scenario 4: Performance RegressionRisk: The mock might hide performance issues in the real jiti import path
Scenario 5: Testing Environment DifferencesRisk: Different Node.js versions might handle module loading differently
Security Considerations✅ No security concerns identified. The changes are purely test-related and don't affect runtime behavior for end users. Performance Considerations✅ Significant improvement: Integration tests are 38x faster (3.8s → 0.1s) Recommendations
Checklist Review
ConclusionThis is excellent work! The PR successfully resolves the coverage instability issue with a clean, maintainable solution. The dependency injection pattern is well-implemented, thoroughly tested, and follows the project's existing conventions. The performance improvements are a bonus. Recommendation: Approve and merge after considering the minor suggestions above (all optional). Great job on the comprehensive testing and clear documentation of the solution! 🎉 |
PR Review: Test Coverage Stability FixSummaryThis PR successfully addresses test coverage instability by implementing dependency injection to prevent jiti double instrumentation. The approach is sound and well-executed. Approval recommended with minor suggestions below. ✅ Strengths
📋 Code Qualitysrc/config/configLoad.ts
Test Files
Fixture Files
🔍 Premortem AnalysisIntegration Test False Positives (Risk: Medium)
Coverage Regression (Risk: Low)
Deployment Risk (Risk: Very Low)
🐛 Potential IssuesMinor Issues Found:
🔒 Security✅ No security issues identified
⚡ Performance✅ Excellent improvements
🧪 Test CoverageOverall: Excellent New coverage includes 625+ lines across cliSpinner, initAction, cliReport, permissionCheck, outputGenerate, and configLoad tests. Test quality is high with clear BDD-style naming, proper setup/teardown, and clean mock management. 📝 Suggestions
✅ Checklist
🎯 Final RecommendationAPPROVE ✅ Excellent work solving a real problem with minimal risk. The DI pattern is clean, test coverage is comprehensive, and performance improvements are substantial. Minor suggestions above are optional refinements, not blockers. Confidence: High | Risk: Low | Merge Readiness: Ready after optional smoke test Great job on the 38x test speedup! 🚀 |
Summary
This PR resolves test coverage instability that occurred after implementing JS/TS config support. Coverage was fluctuating between 68-71% on each test run.
Root Cause
Test fixture files imported
defineConfigfromsrc/index.js, which caused jiti to dynamically transform the entire src/ directory. This created a double instrumentation problem:Solution
loadFileConfigwithdefaultJitiImportfactory for better testabilitydefineConfigimports from all test fixtures - now use plain objectsResults
Changes
Modified Files:
src/config/configLoad.ts- Added DI pattern and defaultJitiImport factorytests/config/configLoad.integration.test.ts- Mock jiti for dynamic config testsReverted:
Checklist
npm run testnpm run lint