feat: show skipped requests with parsing errors in report#6780
Conversation
WalkthroughReads Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI Runner
participant Global as global.brunoSkippedFiles
participant Utils as createSkippedFileResults
participant Reporter as HTML Report Generator
CLI->>Global: read brunoSkippedFiles
CLI->>Utils: createSkippedFileResults(skippedFiles, collectionPath)
Utils-->>CLI: skippedResults (path, name, suitename, status:'skipped', error)
CLI->>CLI: augment regular results with path/name/suitename
CLI->>CLI: append skippedResults to results array
CLI->>Reporter: pass final results to HTML report generator
Reporter->>Reporter: compute hasError (includes skipped+error)
Reporter->>User: render report (suppress "request failed" for pure skips)
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/bruno-common/src/runner/reports/html/template.ts (2)
760-760: Redundant condition inhasErrorcomputed property.The third part
(props?.result?.response?.status === 'skipped' && props?.result?.error)is redundant. Ifprops?.result?.erroris truthy, the first condition!!props?.result?.erroralready evaluates totrue, making the entire expression true regardless of the skipped status check.♻️ Suggested simplification
- const hasError = computed(() => !!props?.result?.error || props?.result?.status === 'error' || (props?.result?.response?.status === 'skipped' && props?.result?.error)); + const hasError = computed(() => !!props?.result?.error || props?.result?.status === 'error');
368-370: Redundant condition in v-if directive.Given the
hasErrorcomputed property already returnstruewhenresult.errorexists (line 760), the additional check(result.response.status === 'skipped' && result.error)is unnecessary—ifresult.erroris truthy,hasErrorwill already betrue.♻️ Suggested simplification
- <n-alert v-if="hasError || (result.response.status === 'skipped' && result.error)" title="Error" type="error"> + <n-alert v-if="hasError" title="Error" type="error">
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/bruno-cli/src/commands/run.jspackages/bruno-common/src/runner/reports/html/template.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-cli/src/commands/run.jspackages/bruno-common/src/runner/reports/html/template.ts
🧠 Learnings (2)
📚 Learning: 2025-12-16T07:16:23.647Z
Learnt from: sanish-bruno
Repo: usebruno/bruno PR: 6090
File: tests/scripting/hooks/init-user-data/ui-state-snapshot.json:1-8
Timestamp: 2025-12-16T07:16:23.647Z
Learning: For e2e tests in the bruno repository: Collections that are shared between CLI and UI tests (comprehensive test suites testing core functionality) should be placed in `packages/bruno-tests/` to avoid duplication. The `tests/**/fixtures/collection` pattern should be used for test-specific collections that test particular UI behaviors or are specific to a single test file.
Applied to files:
packages/bruno-cli/src/commands/run.js
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-cli/src/commands/run.js
🧬 Code graph analysis (1)
packages/bruno-cli/src/commands/run.js (1)
packages/bruno-cli/src/utils/filesystem.js (2)
stripExtension(102-104)results(84-84)
⏰ 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). (6)
- GitHub Check: Unit Tests
- GitHub Check: CLI Tests
- GitHub Check: Playwright E2E Tests
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - Linux
🔇 Additional comments (4)
packages/bruno-cli/src/commands/run.js (4)
696-698: LGTM!Adding
nameandpathfields to regular results ensures consistency with the new skipped result structure.
770-771: Appending skipped results at the end is appropriate.This placement ensures all executed requests are processed first, and skipped entries appear at the end of the results array. This ordering makes sense for both summary generation and report display.
7-7: Import addition is correct.The
stripExtensionutility is properly imported and used at line 626 to derive the display name for skipped files.
600-630: Implementation is solid;global.brunoSkippedFilesis properly populated and path handling is safe.
global.brunoSkippedFilesgets initialized and populated incollection.js(lines 76-77) when file parsing fails during collection traversal. Since skipped files are discovered during the traversal starting fromcollectionPath,skippedFile.pathis guaranteed to be under that root—path.relative()won't produce unexpected relative paths.The result object structure aligns with regular results and includes all necessary fields. The implementation handles the global variable safely with
|| []initialization.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/bruno-cli/src/commands/run.js (2)
600-634: Consider defensive guard forskippedFile.path.If a malformed entry in
global.brunoSkippedFileslacks apathproperty, bothpath.relative()(line 602) andpath.basename()(line 631) will throw. While upstream code should guarantee the shape, a guard would improve resilience.♻️ Suggested defensive filter
const skippedFiles = global.brunoSkippedFiles || []; -const skippedResults = skippedFiles.map((skippedFile) => { +const skippedResults = skippedFiles + .filter((skippedFile) => skippedFile?.path) + .map((skippedFile) => {
600-600: Consider clearing the global after consumption.If the CLI handler runs multiple times within a single process (e.g., during integration tests),
global.brunoSkippedFilesmay retain stale entries. Clearing it after reading prevents accumulation.♻️ Optional cleanup
const skippedFiles = global.brunoSkippedFiles || []; +global.brunoSkippedFiles = [];
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/bruno-cli/src/commands/run.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-cli/src/commands/run.js
🧠 Learnings (2)
📚 Learning: 2025-12-16T07:16:23.647Z
Learnt from: sanish-bruno
Repo: usebruno/bruno PR: 6090
File: tests/scripting/hooks/init-user-data/ui-state-snapshot.json:1-8
Timestamp: 2025-12-16T07:16:23.647Z
Learning: For e2e tests in the bruno repository: Collections that are shared between CLI and UI tests (comprehensive test suites testing core functionality) should be placed in `packages/bruno-tests/` to avoid duplication. The `tests/**/fixtures/collection` pattern should be used for test-specific collections that test particular UI behaviors or are specific to a single test file.
Applied to files:
packages/bruno-cli/src/commands/run.js
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-cli/src/commands/run.js
⏰ 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). (6)
- GitHub Check: Unit Tests
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - macOS
- GitHub Check: CLI Tests
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (3)
packages/bruno-cli/src/commands/run.js (3)
7-7: LGTM!Import addition is appropriate;
stripExtensionis correctly sourced from filesystem utilities and used for suitename derivation.
700-701: LGTM!The
pathderivation correctly prioritizesresult.test?.filenamewith a sensible fallback. Consistent with the skipped result structure.
774-775: LGTM!Skipped results are correctly appended after the main run loop completes and before summary generation, ensuring they're included in all reporters.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/bruno-common/src/runner/reports/html/template.ts (2)
368-370: Redundant condition in v-if directive.The condition
(result.response.status === 'skipped' && result.error)is redundant here becausehasErroron line 760 already accounts for this case. Ifresult.erroris truthy,hasErrorwill betruevia its first condition.♻️ Suggested simplification
- <n-alert v-if="hasError || (result.response.status === 'skipped' && result.error)" title="Error" type="error"> + <n-alert v-if="hasError" title="Error" type="error">
760-760: Redundant third condition in hasError computation.The third condition
(props?.result?.response?.status === 'skipped' && props?.result?.error)is logically redundant. Ifprops?.result?.erroris truthy, the first condition!!props?.result?.erroralready evaluates totrue, making the third condition unreachable in practice.♻️ Suggested simplification
- const hasError = computed(() => !!props?.result?.error || props?.result?.status === 'error' || (props?.result?.response?.status === 'skipped' && props?.result?.error)); + const hasError = computed(() => !!props?.result?.error || props?.result?.status === 'error');
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/bruno-common/src/runner/reports/html/template.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-common/src/runner/reports/html/template.ts
⏰ 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). (6)
- GitHub Check: Playwright E2E Tests
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - Linux
- GitHub Check: CLI Tests
- GitHub Check: Unit Tests
- GitHub Check: SSL Tests - macOS
🔇 Additional comments (1)
packages/bruno-common/src/runner/reports/html/template.ts (1)
313-313: LGTM! Clear handling of skipped request display.The conditional logic properly differentiates between skipped requests (showing "Request Skipped") and regular results (showing pass counts), while correctly suppressing the "(request failed)" suffix for skipped items.
…th parsing errors
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/bruno-cli/tests/runner/report-metadata.spec.js (1)
53-53: Unnecessaryasynckeyword.No
awaitis used in this test. The existing test on line 5 has the same issue. Consider removingasyncfrom both if no asynchronous operations are needed.♻️ Suggested fix
- it('should include skipped requests with parsing errors in the HTML report', async () => { + it('should include skipped requests with parsing errors in the HTML report', () => {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/bruno-cli/tests/runner/report-metadata.spec.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-cli/tests/runner/report-metadata.spec.js
🧠 Learnings (4)
📚 Learning: 2025-12-16T07:16:23.647Z
Learnt from: sanish-bruno
Repo: usebruno/bruno PR: 6090
File: tests/scripting/hooks/init-user-data/ui-state-snapshot.json:1-8
Timestamp: 2025-12-16T07:16:23.647Z
Learning: For e2e tests in the bruno repository: Collections that are shared between CLI and UI tests (comprehensive test suites testing core functionality) should be placed in `packages/bruno-tests/` to avoid duplication. The `tests/**/fixtures/collection` pattern should be used for test-specific collections that test particular UI behaviors or are specific to a single test file.
Applied to files:
packages/bruno-cli/tests/runner/report-metadata.spec.js
📚 Learning: 2026-01-12T17:43:53.540Z
Learnt from: sanish-bruno
Repo: usebruno/bruno PR: 6761
File: packages/bruno-converters/src/utils/bru-to-pm-translator.js:294-310
Timestamp: 2026-01-12T17:43:53.540Z
Learning: In the Bruno converters package (packages/bruno-converters), specifically in the bru-to-pm translator: transforming bare `test()` and `expect()` calls to `pm.test()` and `pm.expect()` is acceptable even if it could cause false positives in rare cases where user scripts define their own unrelated `test` or `expect` functions. The common case outweighs the edge case.
Applied to files:
packages/bruno-cli/tests/runner/report-metadata.spec.js
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.test.{js,jsx,ts,tsx} : Add tests for any new functionality or meaningful changes. If code is added, removed, or significantly modified, corresponding tests should be updated or created
Applied to files:
packages/bruno-cli/tests/runner/report-metadata.spec.js
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-cli/tests/runner/report-metadata.spec.js
🧬 Code graph analysis (1)
packages/bruno-cli/tests/runner/report-metadata.spec.js (2)
packages/bruno-cli/src/reporters/html.js (1)
htmlString(20-25)packages/bruno-common/src/runner/reports/html/generate-report.ts (1)
generateHtmlReport(49-49)
⏰ 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). (6)
- GitHub Check: Unit Tests
- GitHub Check: CLI Tests
- GitHub Check: Playwright E2E Tests
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - macOS
🔇 Additional comments (3)
packages/bruno-cli/tests/runner/report-metadata.spec.js (3)
109-111: Assertions are consistent with existing test patterns.Line 111 asserts on template source code (
result.response.status === 'skipped'), which is fragile but matches the pattern used in lines 45-50 where template placeholders are checked. This approach tests template structure rather than rendered output. Acceptable given consistency, but consider adding a comment explaining why you're testing raw template content.
86-98: Summary counts look correct for a skipped request scenario.
skippedRequests: 1with all other counts at 0 correctly represents the test scenario. Good alignment with the PR objective of surfacing skipped requests in reports.
57-84: Addsuitenamefield to mock data for structural accuracy.The actual skipped result construction in
run.js(line 630) includessuitename: stripExtension(relativePath), but this field is missing from the mock data. While the HTML template doesn't currently use this field, the mock should reflect the real result structure for test completeness and future-proofing.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/bruno-cli/src/utils/run.js (1)
4-37: Well-structured utility function for creating skipped result objects.The function correctly:
- Computes relative paths for consistent reporting
- Builds a complete result object matching the expected structure for the HTML report
- Distinguishes
name(with extension) fromsuitename(without extension)One optional improvement: consider adding a brief JSDoc comment to document the expected shape of
skippedFile(requirespathanderrorproperties). This aids future maintainers.📝 Optional: Add JSDoc documentation
+/** + * Creates a standardized result object for a skipped request file. + * @param {{ path: string, error: string }} skippedFile - The skipped file info + * @param {string} collectionPath - The base collection path + * @returns {object} A result object representing the skipped test + */ const createSkippedResult = (skippedFile, collectionPath) => {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/bruno-cli/src/commands/run.jspackages/bruno-cli/src/utils/run.js
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bruno-cli/src/commands/run.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-cli/src/utils/run.js
🧠 Learnings (2)
📚 Learning: 2025-12-16T07:16:23.647Z
Learnt from: sanish-bruno
Repo: usebruno/bruno PR: 6090
File: tests/scripting/hooks/init-user-data/ui-state-snapshot.json:1-8
Timestamp: 2025-12-16T07:16:23.647Z
Learning: For e2e tests in the bruno repository: Collections that are shared between CLI and UI tests (comprehensive test suites testing core functionality) should be placed in `packages/bruno-tests/` to avoid duplication. The `tests/**/fixtures/collection` pattern should be used for test-specific collections that test particular UI behaviors or are specific to a single test file.
Applied to files:
packages/bruno-cli/src/utils/run.js
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-cli/src/utils/run.js
⏰ 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). (6)
- GitHub Check: CLI Tests
- GitHub Check: Unit Tests
- GitHub Check: Playwright E2E Tests
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Linux
🔇 Additional comments (2)
packages/bruno-cli/src/utils/run.js (2)
1-2: LGTM!Clean imports. Using Node's
pathmodule and the localstripExtensionutility is appropriate for the path manipulation needs here.
39-41: LGTM!Export is clean and follows the project's module pattern.
… additional metadata
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/bruno-cli/src/utils/run.js (1)
3-29: Add defensive check for undefined/null inputs.If
skippedFileorskippedFile.pathis undefined,path.relative()will throw. Consider adding a guard:🛡️ Suggested defensive check
const createSkippedResult = (skippedFile, collectionPath) => { + if (!skippedFile?.path) { + return null; + } const relativePath = path.relative(collectionPath, skippedFile.path);Optionally, add a JSDoc comment to document the expected shape of
skippedFile:/** * @param {{ path: string, error: string }} skippedFile * @param {string} collectionPath */packages/bruno-cli/src/commands/run.js (2)
739-750: RedundantrelativePathcomputation and minor inconsistency inname.
relativePathis computed both insidecreateSkippedResult(stored intest.filename) and again on line 742. Consider reusingresult.test.filenameinstead.
nameon line 747 includes the file extension (path.basename(skippedFile.path)), whereas regular request names (line 665) typically don't. This may cause inconsistent display in reports.♻️ Proposed fix
const skippedFiles = global.brunoSkippedFiles || []; skippedFiles.forEach((skippedFile) => { const result = createSkippedResult(skippedFile, collectionPath); - const relativePath = path.relative(collectionPath, skippedFile.path); + const relativePath = result.test.filename; results.push({ ...result, runDuration: 0, suitename: stripExtension(relativePath), - name: path.basename(skippedFile.path), + name: stripExtension(path.basename(skippedFile.path)), path: relativePath }); });
664-664: Inconsistent extension stripping.Line 664 uses
.replace('.bru', '')while line 746 usesstripExtension(). Consider usingstripExtensionhere as well for consistency and to handle non-.bru formats correctly.♻️ Suggested change
- suitename: pathname.replace('.bru', ''), + suitename: stripExtension(pathname),
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/bruno-cli/src/commands/run.jspackages/bruno-cli/src/utils/run.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-cli/src/utils/run.jspackages/bruno-cli/src/commands/run.js
🧠 Learnings (2)
📚 Learning: 2025-12-16T07:16:23.647Z
Learnt from: sanish-bruno
Repo: usebruno/bruno PR: 6090
File: tests/scripting/hooks/init-user-data/ui-state-snapshot.json:1-8
Timestamp: 2025-12-16T07:16:23.647Z
Learning: For e2e tests in the bruno repository: Collections that are shared between CLI and UI tests (comprehensive test suites testing core functionality) should be placed in `packages/bruno-tests/` to avoid duplication. The `tests/**/fixtures/collection` pattern should be used for test-specific collections that test particular UI behaviors or are specific to a single test file.
Applied to files:
packages/bruno-cli/src/utils/run.jspackages/bruno-cli/src/commands/run.js
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-cli/src/utils/run.jspackages/bruno-cli/src/commands/run.js
⏰ 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). (6)
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - macOS
- GitHub Check: Unit Tests
- GitHub Check: CLI Tests
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (2)
packages/bruno-cli/src/commands/run.js (2)
665-667: LGTM on addingpathfield.The addition of
nameandpathto results provides useful metadata for reporting. The fallback on line 666 ensures correctness.
7-7: LGTM on imports.New imports are correctly added and utilized.
Also applies to: 20-20
…ed skipped file processing
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/bruno-cli/src/commands/run.js (1)
7-7: Remove unused import:stripExtension.The import on line 7 is unused in this file. Remove it from the destructuring assignment.
-const { exists, isFile, isDirectory, stripExtension } = require('../utils/filesystem'); +const { exists, isFile, isDirectory } = require('../utils/filesystem');
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/bruno-cli/src/commands/run.jspackages/bruno-cli/src/utils/run.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-cli/src/commands/run.jspackages/bruno-cli/src/utils/run.js
🧠 Learnings (2)
📚 Learning: 2025-12-16T07:16:23.647Z
Learnt from: sanish-bruno
Repo: usebruno/bruno PR: 6090
File: tests/scripting/hooks/init-user-data/ui-state-snapshot.json:1-8
Timestamp: 2025-12-16T07:16:23.647Z
Learning: For e2e tests in the bruno repository: Collections that are shared between CLI and UI tests (comprehensive test suites testing core functionality) should be placed in `packages/bruno-tests/` to avoid duplication. The `tests/**/fixtures/collection` pattern should be used for test-specific collections that test particular UI behaviors or are specific to a single test file.
Applied to files:
packages/bruno-cli/src/commands/run.jspackages/bruno-cli/src/utils/run.js
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-cli/src/commands/run.jspackages/bruno-cli/src/utils/run.js
🧬 Code graph analysis (2)
packages/bruno-cli/src/commands/run.js (2)
packages/bruno-cli/src/utils/run.js (2)
path(1-1)createSkippedResults(4-36)packages/bruno-cli/src/utils/filesystem.js (2)
path(1-1)results(84-84)
packages/bruno-cli/src/utils/run.js (2)
packages/bruno-cli/src/commands/run.js (14)
require(5-5)require(6-6)require(7-7)require(8-8)require(9-9)require(10-10)require(11-11)require(14-14)require(15-15)require(16-16)require(18-18)require(19-19)require(20-20)skippedFiles(739-739)packages/bruno-cli/src/utils/filesystem.js (1)
stripExtension(102-104)
⏰ 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). (6)
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Linux
- GitHub Check: Playwright E2E Tests
- GitHub Check: CLI Tests
- GitHub Check: Unit Tests
🔇 Additional comments (3)
packages/bruno-cli/src/commands/run.js (2)
661-667: LGTM!The addition of the
pathfield provides consistent metadata across both regular and skipped results. The fallback topath.relative(collectionPath, pathname)ensures the field is always populated.
739-741: Clean integration of skipped results.The skipped results are correctly appended before the summary calculation. The
|| []fallback handles the case whereglobal.brunoSkippedFilesis unset.One consideration: using a global variable (
global.brunoSkippedFiles) for passing data between modules is a pragmatic choice for this CLI context, but worth noting as potential tech debt if the codebase moves toward more modular/testable patterns.packages/bruno-cli/src/utils/run.js (1)
4-36: Code is correct; no changes needed.The implementation is clean and properly handles the skipped results. Both concerns in the original review are already addressed:
Null/undefined handling — The caller in
run.jsline 739 usesglobal.brunoSkippedFiles || [], ensuringcreateSkippedResultsalways receives an array, never null or undefined.Property validation — Each skipped file object is guaranteed to have
.pathand.errorproperties because they're set when populatingglobal.brunoSkippedFilesincollection.js(lines 76–77), and the controlled calling pattern prevents malformed entries.The function aligns with coding guidelines and the result shape matches usage elsewhere in the codebase.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/bruno-cli/src/utils/run.js (1)
4-36: Consider defensive handling for malformed skipped file entries.If
skippedFile.pathisundefinedornull,path.relativeandpath.basenamemay produce unexpected results or throw. While the upstream code likely guarantees structure, a guard would make this utility more robust.🛡️ Optional defensive check
const createSkippedFileResults = (skippedFiles, collectionPath) => { - return skippedFiles.map((skippedFile) => { + return skippedFiles.filter((skippedFile) => skippedFile?.path).map((skippedFile) => { const relativePath = path.relative(collectionPath, skippedFile.path);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/bruno-cli/src/commands/run.jspackages/bruno-cli/src/utils/run.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-cli/src/commands/run.jspackages/bruno-cli/src/utils/run.js
🧠 Learnings (2)
📚 Learning: 2025-12-16T07:16:23.647Z
Learnt from: sanish-bruno
Repo: usebruno/bruno PR: 6090
File: tests/scripting/hooks/init-user-data/ui-state-snapshot.json:1-8
Timestamp: 2025-12-16T07:16:23.647Z
Learning: For e2e tests in the bruno repository: Collections that are shared between CLI and UI tests (comprehensive test suites testing core functionality) should be placed in `packages/bruno-tests/` to avoid duplication. The `tests/**/fixtures/collection` pattern should be used for test-specific collections that test particular UI behaviors or are specific to a single test file.
Applied to files:
packages/bruno-cli/src/commands/run.jspackages/bruno-cli/src/utils/run.js
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-cli/src/commands/run.jspackages/bruno-cli/src/utils/run.js
🧬 Code graph analysis (2)
packages/bruno-cli/src/commands/run.js (1)
packages/bruno-cli/src/utils/run.js (2)
path(1-1)createSkippedFileResults(4-36)
packages/bruno-cli/src/utils/run.js (2)
packages/bruno-cli/src/commands/run.js (14)
path(3-3)require(5-5)require(6-6)require(7-7)require(8-8)require(9-9)require(10-10)require(11-11)require(14-14)require(15-15)require(16-16)require(18-18)require(19-19)require(20-20)packages/bruno-cli/src/utils/filesystem.js (1)
stripExtension(102-104)
⏰ 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). (6)
- GitHub Check: CLI Tests
- GitHub Check: Unit Tests
- GitHub Check: Playwright E2E Tests
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Linux
🔇 Additional comments (5)
packages/bruno-cli/src/utils/run.js (2)
1-2: Imports look correct.Standard Node
pathmodule and localstripExtensionutility are appropriately imported.
38-40: Export is clean and follows module conventions.packages/bruno-cli/src/commands/run.js (3)
20-20: Import correctly added.The new utility import is properly structured and matches the export signature.
665-667: Path field addition looks good.The fallback chain (
result.test?.filename || path.relative(...)) ensures thepathfield is always populated, maintaining consistency with skipped results.
739-741: Skipped results integration is correct and well-placed.Appending skipped file results before
printRunSummaryensures they're included in summary calculations and report outputs. The|| []fallback handles the undefined case cleanly. The use ofglobal.brunoSkippedFilesis consistent with the established pattern inpackages/bruno-cli/src/utils/collection.js, where it's initialized and populated.
) * feat: add support for skipped files in run command and update HTML report template * refactor: enhance skipped file handling in run command * fix: improve error display in HTML report for skipped requests * test: add unit test for HTML report generation of skipped requests with parsing errors * test: update HTML report generation tests to check for skipped request summaries * refactor: extract skipped result creation logic into a separate utility function * refactor: enhance skipped result processing in run command to include additional metadata * refactor: rename and enhance createSkippedResults function for improved skipped file processing * refactor: remove unused stripExtension import from run command * refactor: rename createSkippedResults to createSkippedFileResults for clarity and consistency
BRU-2281
Fixes: #6260
Previously, when a request file contained invalid content, Bruno would log a parsing error to the terminal but omit it from the report, and the request would be silently skipped. After merging this PR, users can now see which requests were skipped and the reasons, reducing false positives in test results.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.