fix(cli-utils): Baseline - create parent directory if needed #4833
Conversation
WalkthroughThis pull request introduces a new test suite for the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
packages/cli-utils/src/__tests__/baseline.test.ts (1)
7-14: Consider adding more test cases with varied data structures.While the current test data is valid, consider adding test cases that cover:
- Empty assets array
- Multiple assets
- Different file types and sizes
- Edge cases (e.g., very long file names, special characters)
const TEST_CASES = [ { name: 'empty assets', data: { assets: [] } }, { name: 'multiple assets', data: { assets: [ { name: 'main.js', size: 12 * 1024 }, { name: 'style.css', size: 5 * 1024 }, { name: 'very-long-filename-with-special-chars!@#$.min.js', size: 1 } ] } } ];packages/cli-utils/src/baseline.ts (1)
47-47: Remove unnecessary empty options object.The empty options object passed to
fs.writeFiledoesn't serve any purpose and can be removed for better code clarity.- return fs.writeFile(baselineFilepath, JSON.stringify(data), {}); + return fs.writeFile(baselineFilepath, JSON.stringify(data));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
packages/cli-utils/src/__tests__/baseline.test.ts(1 hunks)packages/cli-utils/src/baseline.ts(1 hunks)
🔇 Additional comments (4)
packages/cli-utils/src/__tests__/baseline.test.ts (3)
1-5: LGTM! Good test setup with virtual file system.
The use of memfs for testing file operations and mocking of fs/promises is appropriate and follows testing best practices.
16-19: LGTM! Good test isolation.
Resetting the virtual file system before each test ensures proper test isolation.
3-3: Verify the implementation of writeBaseline function.
Let's ensure the actual implementation in baseline.ts handles directory creation correctly.
✅ Verification successful
writeBaseline implementation verified to handle directory creation correctly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the implementation exists and handles directory creation
ast-grep --pattern 'export async function writeBaseline($_, $_) {
$$$
mkdir($$$)
$$$
}'
# Look for related tests or implementations
rg -l "writeBaseline" --type ts
Length of output: 236
Script:
#!/bin/bash
# Extract the implementation of writeBaseline function
ast-grep --pattern 'export async function writeBaseline($_, $_) { $$$ }' packages/cli-utils/src/baseline.ts
# Check for directory creation within the writeBaseline function
rg "mkdir" packages/cli-utils/src/baseline.ts
Length of output: 226
packages/cli-utils/src/baseline.ts (1)
46-46: LGTM! Good practice to ensure directory exists.
Creating the parent directory recursively before writing the file is a robust approach that prevents potential ENOENT errors.
| test('should save baseline', async () => { | ||
| const baselineFilepath = 'new-baseline-dir/baseline.json'; | ||
| await writeBaseline(SOURCE_CURRENT as any, baselineFilepath); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove type assertion and add proper typing.
The as any type assertion should be avoided. Instead, define proper types for the baseline data structure.
interface Asset {
name: string;
size: number;
}
interface BaselineData {
assets: Asset[];
}
const SOURCE_CURRENT: BaselineData = {
// ... existing data
};
await writeBaseline(SOURCE_CURRENT, baselineFilepath);| test('should save baseline', async () => { | ||
| const baselineFilepath = 'new-baseline-dir/baseline.json'; | ||
| await writeBaseline(SOURCE_CURRENT as any, baselineFilepath); | ||
|
|
||
| const baselineFile = await vol.promises.readFile(baselineFilepath); | ||
| expect(JSON.parse(baselineFile.toString())).toEqual(SOURCE_CURRENT); | ||
| }); |
There was a problem hiding this comment.
Add more test cases to verify PR objectives and error handling.
The test suite is missing critical test cases:
- Verify that parent directories are created (main PR objective)
- Test error handling scenarios
- Validate directory existence after write
Add these test cases:
test('should create parent directory if it doesn\'t exist', async () => {
const deepPath = 'deep/nested/directory/baseline.json';
await writeBaseline(SOURCE_CURRENT, deepPath);
const dirExists = await vol.promises.stat('deep/nested/directory');
expect(dirExists.isDirectory()).toBe(true);
});
test('should handle file system errors gracefully', async () => {
// Mock a file system error
vol.promises.mkdir = jest.fn().mockRejectedValue(new Error('Mock FS error'));
await expect(
writeBaseline(SOURCE_CURRENT, 'error/baseline.json')
).rejects.toThrow('Mock FS error');
});
#11663 Bundle Size — 381.32KiB (0%).b564138(current) vs 624e4fa master#11661(baseline) Warning Bundle contains 2 duplicate packages – View duplicate packages Bundle metrics
|
| Current #11663 |
Baseline #11661 |
|
|---|---|---|
 Initial JS |
334.44KiB |
334.44KiB |
| Initial CSS |
46.89KiB |
46.89KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
3 |
3 |
| Assets |
4 |
4 |
| Modules |
699 |
699 |
| Duplicate Modules |
0 |
0 |
| Duplicate Code |
0% |
0% |
| Packages |
39 |
39 |
| Duplicate Packages |
1 |
1 |
Bundle size by type no changes
| Current #11663 |
Baseline #11661 |
|
|---|---|---|
| JS |
334.44KiB |
334.44KiB |
| CSS |
46.89KiB |
46.89KiB |
Bundle analysis report Branch fix-cli-baseline-output Project dashboard
Generated by RelativeCI Documentation Report issue
Summary by CodeRabbit
writeBaselinefunction to ensure its reliability.writeBaselinefunction to create the necessary directory structure before saving files, improving error handling.