fix(import): Import fails for gRPC request without URL in exported collection#8367
Open
sharan-bruno wants to merge 4 commits into
Open
fix(import): Import fails for gRPC request without URL in exported collection#8367sharan-bruno wants to merge 4 commits into
sharan-bruno wants to merge 4 commits into
Conversation
sharan-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno,
naman-bruno,
sid-bruno,
utkarsh-bruno and
vijayh-bruno
as code owners
Contributor
WalkthroughThe Bruno importer now normalizes missing request URLs during request and example transformation. New HTTP and gRPC fixtures omit URLs, and a Playwright spec imports the cases and verifies the collections appear in the sidebar. ChangesMissing-URL Bruno import flow
Sequence Diagram(s)sequenceDiagram
participant Spec as Playwright spec
participant Importer as importCollection
participant Transformer as transformItemsInCollection
participant Utility as valueToString
participant Sidebar as buildCommonLocators
Spec->>Importer: import HTTP, gRPC, or example fixture
Importer->>Transformer: transform request items and examples
Transformer->>Utility: normalize item.request.url
Utility-->>Transformer: string value
Transformer->>Utility: normalize example.request.url
Utility-->>Transformer: string value
Importer-->>Spec: imported collection
Spec->>Sidebar: build collection selector
Sidebar-->>Spec: sidebar locator
Spec->>Sidebar: assert collection is visible
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/bruno-schema/src/collections/index.js`:
- Line 132: The shared request URL schema was made optional in a way that
affects all consumers, not just the intended import paths. Revert the broad
change on requestUrlSchema in collections/index.js, keep the shared schema
strict, and apply .optional() only inside the specific HTTP/gRPC request schema
builders that need URL-less imports, such as the request-type-specific schema
definitions.
In `@tests/import/bruno/import-bruno-request-missing-url.spec.ts`:
- Around line 5-30: The Bruno import tests are split across two tests that
depend on shared state and `test.describe.serial`, which makes them
non-parallel-safe. Update the `cases` loop in
`import-bruno-request-missing-url.spec.ts` so each scenario does the import and
the sidebar visibility assertion in the same `test` callback, using
`importCollection` and `buildCommonLocators` together. After inlining the
assertion, remove the serial-only dependency so each case is self-contained and
follows the Arrange-Act-Assert-Cleanup pattern.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d84c31c0-4d79-40fb-b055-64174744439b
📒 Files selected for processing (4)
packages/bruno-schema/src/collections/index.jstests/import/bruno/fixtures/bruno-grpc-request-missing-url.jsontests/import/bruno/fixtures/bruno-http-request-missing-url.jsontests/import/bruno/import-bruno-request-missing-url.spec.ts
sanish-bruno
suggested changes
Jun 26, 2026
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/import/bruno/import-bruno-request-missing-url.spec.ts (1)
5-29: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMake each fixture case self-contained.
Line 26relies on state created byLine 17, so these cases only pass because the suite is serialised. That breaks parallel-safety and makes the regression flaky. Import and sidebar assertion should live in the same test callback for each case, thentest.describe.serialcan go away.Minimal rewrite
-test.describe.serial('Import Bruno Collection - request with missing URL', () => { +test.describe('Import Bruno Collection - request with missing URL', () => { @@ for (const { fixture, collectionName, type } of cases) { test(`imports a ${type} request without a URL`, async ({ page, createTmpDir }) => { const brunoFile = path.resolve(__dirname, 'fixtures', fixture); const location = await createTmpDir(collectionName); await importCollection(page, brunoFile, location, { expectedCollectionName: collectionName }); - }); - - test(`${type} collection appears in the sidebar after import`, async ({ page }) => { + const locators = buildCommonLocators(page); await expect(locators.sidebar.collection(collectionName)).toBeVisible(); }); } });#!/bin/bash set -euo pipefail file='tests/import/bruno/import-bruno-request-missing-url.spec.ts' printf '\n== Suite structure ==\n' sed -n '1,80p' "$file" printf '\n== Shared-state indicators ==\n' rg -n 'describe\.serial|test\(`imports a .* without a URL`|collection appears in the sidebar after import' "$file"As per coding guidelines, “E2E tests must be parallel-safe” and “Every E2E test should follow the Arrange-Act-Assert-Cleanup shape.” As per path instructions, “Follow best practices for Playwright code and e2e automation.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/import/bruno/import-bruno-request-missing-url.spec.ts` around lines 5 - 29, The Bruno import cases are split across two tests, so the sidebar assertion depends on shared state from the import test and only works because the suite is serial. Move the import and visibility check into the same test callback inside the `cases` loop in `import-bruno-request-missing-url.spec.ts`, using `importCollection` and `buildCommonLocators` together for each `fixture`/`collectionName`/`type`, and then remove the `test.describe.serial` wrapper so each case is self-contained and parallel-safe.Sources: Coding guidelines, Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@tests/import/bruno/import-bruno-request-missing-url.spec.ts`:
- Around line 5-29: The Bruno import cases are split across two tests, so the
sidebar assertion depends on shared state from the import test and only works
because the suite is serial. Move the import and visibility check into the same
test callback inside the `cases` loop in
`import-bruno-request-missing-url.spec.ts`, using `importCollection` and
`buildCommonLocators` together for each `fixture`/`collectionName`/`type`, and
then remove the `test.describe.serial` wrapper so each case is self-contained
and parallel-safe.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0c91250b-ae6a-407a-84da-b29f41d3170e
📒 Files selected for processing (2)
tests/import/bruno/fixtures/bruno-http-example-request-missing-url.jsontests/import/bruno/import-bruno-request-missing-url.spec.ts
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.
Jira: https://usebruno.atlassian.net/browse/BRU-2205
Description
Fix: import fails for requests without a URL
requestUrlSchema from .defined() to .optional() so collections containing gRPC/HTTP requests with an empty URL no longer fail import with items[0].request.url must be defined. Added an E2E regression test covering both request types.
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit