fix: unable to add assertions to a request#6435
Conversation
WalkthroughAdded testability hooks and a new assertion schema: data-testid attributes and a public Changes
Sequence Diagram(s)(omitted — changes are testability/schema additions and test code; no new multi-component sequential control flow requiring diagram) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)tests/**/**.*⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (1)📚 Learning: 2025-12-16T07:16:08.934ZApplied to files:
⏰ 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). (5)
🔇 Additional comments (1)
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: 5
🧹 Nitpick comments (2)
packages/bruno-app/src/components/RequestPane/Assertions/index.js (1)
12-25: Ensure unary operators list stays synchronized with schema.The
unaryOperatorsarray is duplicated across multiple files. This list should match the unary operators subset in the schema'sassertionOperatorsarray. Consider extracting to a shared constant to maintain consistency.Locations found in relevant code snippets:
- This file (lines 12-25)
- packages/bruno-app/src/components/RequestPane/Assertions/AssertionRow/index.js (referenced in relevant snippets)
- packages/bruno-js/src/runtime/assert-runtime.js (referenced in relevant snippets)
packages/bruno-app/src/components/RequestPane/Assertions/AssertionOperator/index.js (1)
37-66: Consider extracting operators list to a shared constant.The operators array is currently duplicated between the AssertionOperator component and
packages/bruno-schema/src/collections/index.js(both contain identical lists of 29 operators). While they're currently synchronized, extracting this to a shared constant would prevent potential drift and reduce duplication. The codebase already uses this pattern elsewhere (e.g.,packages/bruno-app/src/components/Environments/.../constants.js).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/bruno-app/src/components/EditableTable/index.js(5 hunks)packages/bruno-app/src/components/RequestPane/Assertions/AssertionOperator/index.js(1 hunks)packages/bruno-app/src/components/RequestPane/Assertions/index.js(1 hunks)packages/bruno-schema/src/collections/index.js(4 hunks)tests/asserts/add-assertions.spec.ts(1 hunks)tests/asserts/fixtures/collection/bruno.json(1 hunks)tests/asserts/fixtures/collection/environments/Local.bru(1 hunks)tests/asserts/fixtures/collection/ping.bru(1 hunks)tests/asserts/init-user-data/preferences.json(1 hunks)tests/utils/page/actions.ts(2 hunks)tests/utils/page/locators.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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-app/src/components/RequestPane/Assertions/AssertionOperator/index.jspackages/bruno-app/src/components/EditableTable/index.jspackages/bruno-app/src/components/RequestPane/Assertions/index.jspackages/bruno-schema/src/collections/index.jstests/utils/page/actions.tstests/asserts/add-assertions.spec.tstests/utils/page/locators.ts
tests/**/**.*
⚙️ CodeRabbit configuration file
tests/**/**.*: Review the following e2e test code written using the Playwright test library. Ensure that:
Follow best practices for Playwright code and e2e automation
Try to reduce usage of
page.waitForTimeout();in code unless absolutely necessary and the locator cannot be found using existingexpect()playwright callsAvoid using
page.pause()in codeUse locator variables for locators
Avoid using test.only
Use multiple assertions
Promote the use of
test.stepas much as possible so the generated reports are easier to readEnsure that the
fixtureslike the collections are nested inside thefixturesfolderFixture Example*: Here's an example of possible fixture and test pair
. ├── fixtures │ └── collection │ ├── base.bru │ ├── bruno.json │ ├── collection.bru │ ├── ws-test-request-with-headers.bru │ ├── ws-test-request-with-subproto.bru │ └── ws-test-request.bru ├── connection.spec.ts # <- Depends on the collection in ./fixtures/collection ├── headers.spec.ts ├── persistence.spec.ts ├── variable-interpolation │ ├── fixtures │ │ └── collection │ │ ├── environments │ │ ├── bruno.json │ │ └── ws-interpolation-test.bru │ ├── init-user-data │ └── variable-interpolation.spec.ts # <- Depends on the collection in ./variable-interpolation/fixtures/collection └── subproto.spec.ts
Files:
tests/asserts/fixtures/collection/environments/Local.brutests/asserts/fixtures/collection/ping.brutests/asserts/fixtures/collection/bruno.jsontests/utils/page/actions.tstests/asserts/init-user-data/preferences.jsontests/asserts/add-assertions.spec.tstests/utils/page/locators.ts
🧠 Learnings (3)
📚 Learning: 2025-12-16T07:16:08.934Z
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:08.934Z
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:
tests/asserts/fixtures/collection/environments/Local.brutests/asserts/fixtures/collection/ping.brutests/asserts/init-user-data/preferences.jsontests/asserts/add-assertions.spec.ts
📚 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-app/src/components/RequestPane/Assertions/index.jstests/asserts/add-assertions.spec.ts
📚 Learning: 2025-12-16T07:16:08.934Z
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:08.934Z
Learning: For e2e tests in the bruno repository: place shared test collections (CLI/UI) under packages/bruno-tests. Use tests/**/fixtures/collection/*.json for test-specific collections that exercise particular UI behaviors or are confined to a single test file. This helps avoid duplication and keeps UI-related test data organized.
Applied to files:
tests/asserts/fixtures/collection/bruno.json
🧬 Code graph analysis (3)
packages/bruno-app/src/components/RequestPane/Assertions/AssertionOperator/index.js (3)
packages/bruno-app/src/components/RequestPane/Assertions/index.js (1)
operator(38-38)packages/bruno-js/src/runtime/assert-runtime.js (1)
operator(127-127)packages/bruno-app/src/components/RequestPane/Assertions/AssertionRow/index.js (1)
operator(93-93)
tests/utils/page/actions.ts (2)
tests/utils/page/locators.ts (1)
buildCommonLocators(3-143)packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js (2)
saveRequest(135-173)saveRequest(135-173)
tests/asserts/add-assertions.spec.ts (2)
tests/utils/page/actions.ts (10)
openCollectionAndAcceptSandbox(848-848)selectEnvironment(862-862)openRequest(864-864)selectRequestPaneTab(868-868)saveRequest(877-877)closeAllCollections(847-847)addAssertion(874-874)sendRequest(863-863)deleteAssertion(876-876)waitForToastToDisappear(878-878)tests/utils/page/locators.ts (1)
buildCommonLocators(3-143)
⏰ 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: Unit Tests
- GitHub Check: CLI Tests
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - macOS
🔇 Additional comments (16)
packages/bruno-app/src/components/RequestPane/Assertions/AssertionOperator/index.js (1)
84-84: LGTM: Test identifier added for assertions table testing.The addition of
data-testid="assertion-operator-select"enables reliable UI testing through the new test suite.tests/asserts/init-user-data/preferences.json (1)
1-5: LGTM: Test preferences configuration looks appropriate.The preferences file correctly sets up the initial collection path for the assertions test suite.
tests/utils/page/locators.ts (2)
111-121: LGTM: Generic table locator builder enhances test reusability.The
table(testId)method provides a clean, reusable pattern for accessing editable tables throughout the test suite.
126-142: Remove unnecessary .or() fallback locators.The fallback patterns don't provide meaningful resilience. Both
getByTestId('assertion-operator-select')andcell.locator('select')match the same element (the select always has the testId), and bothgetByRole('textbox')andcell.locator('input[type="text"]')match the same textbox. Simplify to use only the primary selectors:
rowOperatorSelect: UsegetByTestId('assertion-operator-select')alonerowExprInput: UsegetByRole('textbox')aloneThis improves clarity and reduces maintenance burden.
⛔ Skipped due to learnings
Learnt from: CR Repo: usebruno/bruno PR: 0 File: CODING_STANDARDS.md:0-0 Timestamp: 2025-12-05T20:31:33.005Z Learning: Use consistent patterns and helper utilities where they improve clarity. Prefer shared test utilities over copy-pasted setup code, but only when it actually reduces complexitypackages/bruno-schema/src/collections/index.js (3)
36-65: LGTM: Assertion operators list matches UI component.The assertion operators array is comprehensive and matches the operators defined in the UI component.
415-415: LGTM: Consistent schema update across all request types.The assertions field has been correctly updated to use
assertionSchemainstead ofkeyValueSchemaacross HTTP, gRPC, and WebSocket request schemas. This ensures consistent validation for all request types.Also applies to: 451-451, 489-489
67-74: Clarify the purpose of nullable + optional for the operator field.The combination of
.nullable()and.optional()is a documented Yup pattern that allows three states: undefined, null, or a valid value. Confirm whether this is intentional (e.g., backward compatibility with legacy assertions) or if a stricter approach would be appropriate (e.g.,.required()with a default value).packages/bruno-app/src/components/RequestPane/Assertions/index.js (1)
166-166: LGTM: Test identifier enables assertions table testing.The
testId="assertions-table"attribute properly instruments the EditableTable component for the new test suite.tests/asserts/fixtures/collection/environments/Local.bru (1)
1-3: LGTM: Environment fixture properly configured for assertions tests.The Local environment fixture correctly defines the test host used by the ping.bru request fixture.
tests/asserts/fixtures/collection/bruno.json (1)
1-6: LGTM: Collection fixture properly structured for assertions tests.The bruno.json file correctly defines the test collection metadata. The fixture placement follows best practices for test-specific collections.
Based on learnings, this is appropriately placed under
tests/**/fixtures/collectionas it's specific to the assertions UI test suite.tests/asserts/fixtures/collection/ping.bru (1)
1-11: LGTM: Request fixture properly configured for assertions testing.The ping.bru fixture defines a simple GET request that will be used to test assertion functionality. The
{{host}}placeholder correctly references the environment variable defined in Local.bru.packages/bruno-app/src/components/EditableTable/index.js (1)
19-20: LGTM! Clean testability instrumentation.The
testIdprop anddata-testidattributes are consistently threaded through all interactive elements. The hierarchical naming scheme (${testId}-row-${rowIndex}-${column.key}) provides stable, predictable selectors for test automation without altering component behavior.Also applies to: 228-309
tests/utils/page/actions.ts (4)
695-699: Clean type definition for assertion inputs.The
AssertionInputtype properly captures the required fields with an optional operator, matching the test usage patterns.
764-801: LGTM! Clean assertion editing without timeouts.The
editAssertionfunction properly uses keyboard interactions and Playwright's built-in wait mechanisms without resorting towaitForTimeout.
829-844: LGTM! Proper toast handling.The
saveRequestfunction correctly waits for the success toast usingexpect(...).toBeVisible(), andwaitForToastToDisappearis appropriately a timeout wrapper (that's its documented purpose).
846-881: Exports properly updated with new assertion helpers.The new assertion functions and
AssertionInputtype are correctly added to the exports, maintaining consistency with the existing export structure.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/asserts/add-assertions.spec.ts (1)
37-238: Note:waitForTimeoutusage already flagged in previous reviews.Multiple uses of
page.waitForTimeout()at lines 37, 50, 60, 130, 192, 203, 233, 238 were already identified in previous review comments. Those remain the primary issues to address by replacing fixed timeouts with state-based waits.
🧹 Nitpick comments (1)
tests/asserts/add-assertions.spec.ts (1)
226-244: Use exact counts instead of minimum thresholds in assertions.Lines 228 and 242 use
toBeGreaterThanOrEqualwhich allows unexpected extra rows to pass undetected. After adding 3 assertions, expect exactly 4 rows (3 data + 1 empty). After deleting 2, expect exactly 2 rows.await test.step('Verify three assertions exist', async () => { const rowCount = await table.allRows().count(); - expect(rowCount).toBeGreaterThanOrEqual(3); + expect(rowCount).toBe(4); // 3 assertions + 1 empty row });await test.step('Verify only one assertion remains', async () => { const rowCount = await table.allRows().count(); - // Should have at least 1 assertion row + 1 empty row - expect(rowCount).toBeGreaterThanOrEqual(1); + expect(rowCount).toBe(2); // 1 assertion + 1 empty row });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/asserts/add-assertions.spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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:
tests/asserts/add-assertions.spec.ts
tests/**/**.*
⚙️ CodeRabbit configuration file
tests/**/**.*: Review the following e2e test code written using the Playwright test library. Ensure that:
Follow best practices for Playwright code and e2e automation
Try to reduce usage of
page.waitForTimeout();in code unless absolutely necessary and the locator cannot be found using existingexpect()playwright callsAvoid using
page.pause()in codeUse locator variables for locators
Avoid using test.only
Use multiple assertions
Promote the use of
test.stepas much as possible so the generated reports are easier to readEnsure that the
fixtureslike the collections are nested inside thefixturesfolderFixture Example*: Here's an example of possible fixture and test pair
. ├── fixtures │ └── collection │ ├── base.bru │ ├── bruno.json │ ├── collection.bru │ ├── ws-test-request-with-headers.bru │ ├── ws-test-request-with-subproto.bru │ └── ws-test-request.bru ├── connection.spec.ts # <- Depends on the collection in ./fixtures/collection ├── headers.spec.ts ├── persistence.spec.ts ├── variable-interpolation │ ├── fixtures │ │ └── collection │ │ ├── environments │ │ ├── bruno.json │ │ └── ws-interpolation-test.bru │ ├── init-user-data │ └── variable-interpolation.spec.ts # <- Depends on the collection in ./variable-interpolation/fixtures/collection └── subproto.spec.ts
Files:
tests/asserts/add-assertions.spec.ts
🧠 Learnings (7)
📓 Common learnings
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
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Use consistent patterns and helper utilities where they improve clarity. Prefer shared test utilities over copy-pasted setup code, but only when it actually reduces complexity
📚 Learning: 2025-12-16T07:16:08.934Z
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:08.934Z
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:
tests/asserts/add-assertions.spec.ts
📚 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:
tests/asserts/add-assertions.spec.ts
📚 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: Use consistent patterns and helper utilities where they improve clarity. Prefer shared test utilities over copy-pasted setup code, but only when it actually reduces complexity
Applied to files:
tests/asserts/add-assertions.spec.ts
📚 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: Tests should be fast enough to run continuously. Avoid long-running operations unless absolutely necessary; prefer lightweight fixtures and isolated units
Applied to files:
tests/asserts/add-assertions.spec.ts
📚 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: Keep tests readable and maintainable. Optimise for clarity over cleverness. Name tests descriptively, keep setup minimal, and avoid unnecessary abstraction
Applied to files:
tests/asserts/add-assertions.spec.ts
📚 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: Avoid overfitting tests to current behaviour if future flexibility matters. Only assert what needs to be true, not incidental details
Applied to files:
tests/asserts/add-assertions.spec.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: SSL Tests - Windows
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - macOS
- GitHub Check: CLI Tests
- GitHub Check: Playwright E2E Tests
- GitHub Check: Unit Tests
🔇 Additional comments (3)
tests/asserts/add-assertions.spec.ts (3)
1-26: Excellent test structure and navigation setup.The imports, fixture usage, and beforeAll navigation steps are well-organized. Good use of
test.stepfor clear test reporting.
67-101: Comprehensive end-to-end test flow.This test properly covers the full lifecycle: add → save → send → verify → delete → save. The response verification checks both status code and body content appropriately.
252-289: Good operator coverage.The test properly validates assertions work with different operators (contains, gt, length) and verifies the expressions are persisted correctly.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
tests/utils/page/actions.ts (2)
807-819: Add verification that the row was actually deleted.The function clicks the delete button but doesn't verify the row was removed from the DOM. This was previously flagged.
Based on coding guidelines and past review. Add after line 817:
const deleteButton = table.rowDeleteButton(rowIndex); await deleteButton.click(); + + // Verify the row was removed + await expect(table.row(rowIndex)).not.toBeVisible({ timeout: 2000 }); }); };
707-753: Replace timeout with state-based wait at line 737.The function correctly uses state-based waits at lines 731 and 749, but line 737 still uses
page.waitForTimeout(300). Per coding guidelines, prefer explicit waits for DOM state.Based on coding guidelines. Replace with:
- await page.waitForTimeout(300); + // Wait for value to be reflected in the DOM + await expect(valueInput).toHaveValue(assertion.value, { timeout: 1000 });Note: The valueInput is a SingleLineEditor component; if the above doesn't work, wait for the combined operator+value text to appear in the cell.
🧹 Nitpick comments (2)
tests/utils/page/actions.ts (2)
762-799: Consider adding verification after editing assertion fields.The function updates expression, operator, and value but doesn't verify the changes were applied. For better test reliability, add assertions to confirm the updated values are reflected in the DOM.
Add after line 797:
await page.keyboard.press('Backspace'); // Clear selection await page.keyboard.type(assertion.value); + + // Verify the changes were applied + await expect(exprInput).toHaveValue(assertion.expr, { timeout: 1000 }); + await expect(valueInput).toHaveValue(assertion.value, { timeout: 1000 }); }); };
826-832: Consider removing or justifying the timeout at line 830.The function already waits for the success toast, which confirms the save completed. The additional
waitForTimeout(200)may be unnecessary.Based on coding guidelines:
await page.keyboard.press('Meta+s'); await expect(page.getByText('Request saved successfully').last()).toBeVisible({ timeout: 3000 }); - await page.waitForTimeout(200); }); };If the timeout is needed for the toast to disappear or for downstream state to settle, add a comment explaining why.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/asserts/add-assertions.spec.ts(1 hunks)tests/utils/page/actions.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/asserts/add-assertions.spec.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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:
tests/utils/page/actions.ts
tests/**/**.*
⚙️ CodeRabbit configuration file
tests/**/**.*: Review the following e2e test code written using the Playwright test library. Ensure that:
Follow best practices for Playwright code and e2e automation
Try to reduce usage of
page.waitForTimeout();in code unless absolutely necessary and the locator cannot be found using existingexpect()playwright callsAvoid using
page.pause()in codeUse locator variables for locators
Avoid using test.only
Use multiple assertions
Promote the use of
test.stepas much as possible so the generated reports are easier to readEnsure that the
fixtureslike the collections are nested inside thefixturesfolderFixture Example*: Here's an example of possible fixture and test pair
. ├── fixtures │ └── collection │ ├── base.bru │ ├── bruno.json │ ├── collection.bru │ ├── ws-test-request-with-headers.bru │ ├── ws-test-request-with-subproto.bru │ └── ws-test-request.bru ├── connection.spec.ts # <- Depends on the collection in ./fixtures/collection ├── headers.spec.ts ├── persistence.spec.ts ├── variable-interpolation │ ├── fixtures │ │ └── collection │ │ ├── environments │ │ ├── bruno.json │ │ └── ws-interpolation-test.bru │ ├── init-user-data │ └── variable-interpolation.spec.ts # <- Depends on the collection in ./variable-interpolation/fixtures/collection └── subproto.spec.ts
Files:
tests/utils/page/actions.ts
🧠 Learnings (1)
📓 Common learnings
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
🧬 Code graph analysis (1)
tests/utils/page/actions.ts (3)
packages/bruno-app/src/components/RequestPane/Assertions/index.js (1)
operator(38-38)tests/utils/page/locators.ts (1)
buildCommonLocators(3-143)packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js (2)
saveRequest(135-173)saveRequest(135-173)
⏰ 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 (2)
tests/utils/page/actions.ts (2)
695-699: LGTM!Clean type definition for assertion inputs. The optional operator field is appropriate given the default handling in the action functions.
861-868: LGTM!All new assertion utilities and types are properly exported, following the existing pattern in the file.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/utils/page/actions.ts (2)
761-796: Consider adding verification of edited values.The edit logic is sound and follows best practices with triple-click selection and state-based waits. However, consider adding assertions at the end to verify the edited values were actually saved.
Add verification after line 794:
await page.keyboard.type(assertion.value); + + // Verify the edit was successful + await expect(exprInput).toHaveValue(assertion.expr, { timeout: 2000 }); + await expect(valueInput).toHaveValue(assertion.value, { timeout: 2000 }); }); };
824-830: Reduce arbitrary timeout after save verification.The save logic is correct, but line 828 uses
waitForTimeout(200)after the toast is already verified. Per coding guidelines, prefer state-based waits.Based on coding guidelines:
await page.keyboard.press('Meta+s'); await expect(page.getByText('Request saved successfully').last()).toBeVisible({ timeout: 3000 }); - await page.waitForTimeout(200); + // Toast verification is sufficient; UI should be ready for next action }); };If additional settling is needed, wait for a specific UI state (e.g., toast disappearing or a save indicator).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/utils/page/actions.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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:
tests/utils/page/actions.ts
tests/**/**.*
⚙️ CodeRabbit configuration file
tests/**/**.*: Review the following e2e test code written using the Playwright test library. Ensure that:
Follow best practices for Playwright code and e2e automation
Try to reduce usage of
page.waitForTimeout();in code unless absolutely necessary and the locator cannot be found using existingexpect()playwright callsAvoid using
page.pause()in codeUse locator variables for locators
Avoid using test.only
Use multiple assertions
Promote the use of
test.stepas much as possible so the generated reports are easier to readEnsure that the
fixtureslike the collections are nested inside thefixturesfolderFixture Example*: Here's an example of possible fixture and test pair
. ├── fixtures │ └── collection │ ├── base.bru │ ├── bruno.json │ ├── collection.bru │ ├── ws-test-request-with-headers.bru │ ├── ws-test-request-with-subproto.bru │ └── ws-test-request.bru ├── connection.spec.ts # <- Depends on the collection in ./fixtures/collection ├── headers.spec.ts ├── persistence.spec.ts ├── variable-interpolation │ ├── fixtures │ │ └── collection │ │ ├── environments │ │ ├── bruno.json │ │ └── ws-interpolation-test.bru │ ├── init-user-data │ └── variable-interpolation.spec.ts # <- Depends on the collection in ./variable-interpolation/fixtures/collection └── subproto.spec.ts
Files:
tests/utils/page/actions.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: SSL Tests - Linux
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - macOS
- GitHub Check: Unit Tests
- GitHub Check: CLI Tests
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (4)
tests/utils/page/actions.ts (4)
695-699: LGTM! Clean type definition.The
AssertionInputtype is well-structured with appropriate optional field for operator.
707-752: LGTM! Well-implemented with proper state-based waits.The function correctly uses
expectassertions instead of arbitrary timeouts (line 731 for row count, line 748 for verification). Past feedback has been properly addressed. The use oftest.stepand multiple locator-based assertions aligns with Playwright best practices.
804-817: LGTM! Clean deletion with proper verification.The function correctly uses
expectto verify the row was removed (line 815) instead of arbitrary timeouts. Past feedback has been addressed.
859-866: LGTM! Module exports properly updated.All new assertion functions and the
AssertionInputtype are correctly exported, maintaining a clean public API.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tests/utils/page/locators.ts (1)
134-136: Avoid recursive call tobuildCommonLocatorsinside itself.Line 135 calls
buildCommonLocators(page).table('assertions-table'), but we're already insidebuildCommonLocators. This creates a new locators object unnecessarily. Reference the localtablefunction directly instead.🔎 Suggested fix:
assertionsTable: () => { - const baseTable = buildCommonLocators(page).table('assertions-table'); + const tableLocators = { + container: () => page.getByTestId('assertions-table'), + row: (index?: number) => { + const locator = page.getByTestId('assertions-table').locator('tbody tr'); + return index !== undefined ? locator.nth(index) : locator; + }, + rowCell: (columnKey: string, rowIndex?: number) => { + const locator = page.getByTestId('assertions-table').locator('tbody tr'); + const row = rowIndex !== undefined ? locator.nth(rowIndex) : locator; + return row.getByTestId(`column-${columnKey}`); + }, + rowCheckbox: (rowIndex: number) => page.getByTestId('assertions-table').locator('tbody tr').nth(rowIndex).getByTestId('column-checkbox'), + rowDeleteButton: (rowIndex: number) => page.getByTestId('assertions-table').locator('tbody tr').nth(rowIndex).getByTestId('column-delete'), + allRows: () => page.getByTestId('assertions-table').locator('tbody tr') + }; return { - ...baseTable, + ...tableLocators,Alternatively, extract
tableas a standalone function outside the returned object and call it directly here.tests/asserts/add-assertions.spec.ts (2)
153-158: Verification approach is acceptable but could be more precise.Using
toContainTexton the value cell works, but if the value cell contains an input, consider using the locator'srowValueInput(0)and checking the input value directly for consistency with line 154.
216-232: Consider using exact row counts for stronger assertions.
toBeGreaterThanOrEqualis imprecise. After adding 3 assertions, you expect exactly 4 rows (3 data + 1 empty). After deleting 2, exactly 2 rows remain. UsingtoHaveCount()would make test failures more informative.🔎 Suggested fix:
await test.step('Verify three assertions exist', async () => { - const rowCount = await table.allRows().count(); - expect(rowCount).toBeGreaterThanOrEqual(3); + // 3 data rows + 1 empty row = 4 rows + await expect(table.allRows()).toHaveCount(4); }); // ... deletion steps ... await test.step('Verify only one assertion remains', async () => { - const rowCount = await table.allRows().count(); - // Should have at least 1 assertion row + 1 empty row - expect(rowCount).toBeGreaterThanOrEqual(1); + // 1 data row + 1 empty row = 2 rows + await expect(table.allRows()).toHaveCount(2); });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/bruno-app/src/components/EditableTable/index.js(3 hunks)tests/asserts/add-assertions.spec.ts(1 hunks)tests/utils/page/locators.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bruno-app/src/components/EditableTable/index.js
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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:
tests/utils/page/locators.tstests/asserts/add-assertions.spec.ts
tests/**/**.*
⚙️ CodeRabbit configuration file
tests/**/**.*: Review the following e2e test code written using the Playwright test library. Ensure that:
Follow best practices for Playwright code and e2e automation
Try to reduce usage of
page.waitForTimeout();in code unless absolutely necessary and the locator cannot be found using existingexpect()playwright callsAvoid using
page.pause()in codeUse locator variables for locators
Avoid using test.only
Use multiple assertions
Promote the use of
test.stepas much as possible so the generated reports are easier to readEnsure that the
fixtureslike the collections are nested inside thefixturesfolderFixture Example*: Here's an example of possible fixture and test pair
. ├── fixtures │ └── collection │ ├── base.bru │ ├── bruno.json │ ├── collection.bru │ ├── ws-test-request-with-headers.bru │ ├── ws-test-request-with-subproto.bru │ └── ws-test-request.bru ├── connection.spec.ts # <- Depends on the collection in ./fixtures/collection ├── headers.spec.ts ├── persistence.spec.ts ├── variable-interpolation │ ├── fixtures │ │ └── collection │ │ ├── environments │ │ ├── bruno.json │ │ └── ws-interpolation-test.bru │ ├── init-user-data │ └── variable-interpolation.spec.ts # <- Depends on the collection in ./variable-interpolation/fixtures/collection └── subproto.spec.ts
Files:
tests/utils/page/locators.tstests/asserts/add-assertions.spec.ts
🧠 Learnings (6)
📚 Learning: 2025-12-16T07:16:08.934Z
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:08.934Z
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:
tests/asserts/add-assertions.spec.ts
📚 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:
tests/asserts/add-assertions.spec.ts
📚 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: Use consistent patterns and helper utilities where they improve clarity. Prefer shared test utilities over copy-pasted setup code, but only when it actually reduces complexity
Applied to files:
tests/asserts/add-assertions.spec.ts
📚 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: Tests should be fast enough to run continuously. Avoid long-running operations unless absolutely necessary; prefer lightweight fixtures and isolated units
Applied to files:
tests/asserts/add-assertions.spec.ts
📚 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: Avoid overfitting tests to current behaviour if future flexibility matters. Only assert what needs to be true, not incidental details
Applied to files:
tests/asserts/add-assertions.spec.ts
📚 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: Keep tests readable and maintainable. Optimise for clarity over cleverness. Name tests descriptively, keep setup minimal, and avoid unnecessary abstraction
Applied to files:
tests/asserts/add-assertions.spec.ts
🔇 Additional comments (7)
tests/utils/page/locators.ts (1)
139-149: Well-structured assertion-specific helpers.The use of
.or()for fallback locators is a solid defensive pattern. The helpers provide a clean abstraction for interacting with assertion row inputs.tests/asserts/add-assertions.spec.ts (6)
28-56: Solid cleanup logic with proper state-based waits.The
afterEachhook correctly handles index shifting by deleting from the end. Usingexpect(table.allRows()).toHaveCount(rowCount - 1)instead ofwaitForTimeoutis the right approach per Playwright best practices.
62-96: Good end-to-end coverage for assertion workflow.The test covers the full lifecycle: add → save → send → verify response → cleanup. Clear step descriptions make the test report readable.
123-128: Clear verification of multiple assertion inputs.Using
toHaveValue()on the input elements is more reliable than checking cell text content. Good approach.
165-204: Comprehensive checkbox toggle coverage.Good pattern: add → verify checked → uncheck → verify unchecked → re-check → verify checked. All state transitions are explicitly verified.
268-272: Operator test covers inputs but not operator verification.The test adds assertions with different operators but only verifies the expression inputs, not the operator selections. Consider adding verification that the operators were correctly set using
table.rowOperatorSelect(n)if operator persistence is critical.
1-15: Clean imports and well-organized test structure.Good separation of concerns with utility functions imported from centralized locations. Test file follows e2e best practices.
bcc22c9 to
8fc2599
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/bruno-schema/src/collections/index.js (1)
68-71: Simplify operator field definition.The operator field uses both
.nullable()and.optional(), which is redundant. In Yup,.nullable()already permitsnullandundefinedvalues.🔎 Simplify to just .nullable():
operator: Yup.string() .oneOf(assertionOperators) .nullable() - .optional()tests/asserts/add-assertions.spec.ts (1)
25-25: Consider replacing fixed timeout with state-based wait.Line 25 uses
page.waitForTimeout(1000)after selecting the Assert tab. If possible, replace with a deterministic wait condition.🔎 Replace with state-based wait:
await selectRequestPaneTab(page, 'Assert'); - await page.waitForTimeout(1000); + const locators = buildCommonLocators(page); + const table = locators.assertionsTable(); + await expect(table.container()).toBeVisible();Based on coding guidelines: "Try to reduce usage of
page.waitForTimeout();in code unless absolutely necessary."packages/bruno-app/src/components/RequestPane/Assertions/AssertionOperator/index.js (1)
37-66: Extract operators array to shared constant to prevent synchronization issues.The operators array defined here is duplicated in
packages/bruno-schema/src/collections/index.jswith identical content. Both maintain the same 28 operators, creating a maintenance risk if either list is updated. Move to a shared constant that both packages can import.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/bruno-app/src/components/EditableTable/index.js(3 hunks)packages/bruno-app/src/components/RequestPane/Assertions/AssertionOperator/index.js(1 hunks)packages/bruno-app/src/components/RequestPane/Assertions/index.js(1 hunks)packages/bruno-schema/src/collections/index.js(4 hunks)tests/asserts/add-assertions.spec.ts(1 hunks)tests/asserts/fixtures/collection/bruno.json(1 hunks)tests/asserts/fixtures/collection/environments/Local.bru(1 hunks)tests/asserts/fixtures/collection/ping.bru(1 hunks)tests/asserts/init-user-data/preferences.json(1 hunks)tests/utils/page/actions.ts(2 hunks)tests/utils/page/locators.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/asserts/fixtures/collection/bruno.json
🚧 Files skipped from review as they are similar to previous changes (5)
- tests/utils/page/locators.ts
- tests/asserts/fixtures/collection/environments/Local.bru
- packages/bruno-app/src/components/RequestPane/Assertions/index.js
- tests/utils/page/actions.ts
- tests/asserts/init-user-data/preferences.json
🧰 Additional context used
📓 Path-based instructions (2)
tests/**/**.*
⚙️ CodeRabbit configuration file
tests/**/**.*: Review the following e2e test code written using the Playwright test library. Ensure that:
Follow best practices for Playwright code and e2e automation
Try to reduce usage of
page.waitForTimeout();in code unless absolutely necessary and the locator cannot be found using existingexpect()playwright callsAvoid using
page.pause()in codeUse locator variables for locators
Avoid using test.only
Use multiple assertions
Promote the use of
test.stepas much as possible so the generated reports are easier to readEnsure that the
fixtureslike the collections are nested inside thefixturesfolderFixture Example*: Here's an example of possible fixture and test pair
. ├── fixtures │ └── collection │ ├── base.bru │ ├── bruno.json │ ├── collection.bru │ ├── ws-test-request-with-headers.bru │ ├── ws-test-request-with-subproto.bru │ └── ws-test-request.bru ├── connection.spec.ts # <- Depends on the collection in ./fixtures/collection ├── headers.spec.ts ├── persistence.spec.ts ├── variable-interpolation │ ├── fixtures │ │ └── collection │ │ ├── environments │ │ ├── bruno.json │ │ └── ws-interpolation-test.bru │ ├── init-user-data │ └── variable-interpolation.spec.ts # <- Depends on the collection in ./variable-interpolation/fixtures/collection └── subproto.spec.ts
Files:
tests/asserts/fixtures/collection/ping.brutests/asserts/add-assertions.spec.ts
**/*.{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-app/src/components/EditableTable/index.jspackages/bruno-schema/src/collections/index.jstests/asserts/add-assertions.spec.tspackages/bruno-app/src/components/RequestPane/Assertions/AssertionOperator/index.js
🧠 Learnings (8)
📓 Common learnings
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
📚 Learning: 2025-12-16T07:16:08.934Z
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:08.934Z
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:
tests/asserts/fixtures/collection/ping.brutests/asserts/add-assertions.spec.ts
📚 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-app/src/components/EditableTable/index.jspackages/bruno-schema/src/collections/index.jspackages/bruno-app/src/components/RequestPane/Assertions/AssertionOperator/index.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:
tests/asserts/add-assertions.spec.ts
📚 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: Use consistent patterns and helper utilities where they improve clarity. Prefer shared test utilities over copy-pasted setup code, but only when it actually reduces complexity
Applied to files:
tests/asserts/add-assertions.spec.ts
📚 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: Tests should be fast enough to run continuously. Avoid long-running operations unless absolutely necessary; prefer lightweight fixtures and isolated units
Applied to files:
tests/asserts/add-assertions.spec.ts
📚 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: Avoid overfitting tests to current behaviour if future flexibility matters. Only assert what needs to be true, not incidental details
Applied to files:
tests/asserts/add-assertions.spec.ts
📚 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: Keep tests readable and maintainable. Optimise for clarity over cleverness. Name tests descriptively, keep setup minimal, and avoid unnecessary abstraction
Applied to files:
tests/asserts/add-assertions.spec.ts
🧬 Code graph analysis (2)
tests/asserts/add-assertions.spec.ts (2)
tests/utils/page/actions.ts (9)
openCollection(847-847)selectEnvironment(861-861)openRequest(863-863)selectRequestPaneTab(867-867)saveRequest(876-876)addAssertion(873-873)sendRequest(862-862)deleteAssertion(875-875)editAssertion(874-874)tests/utils/page/locators.ts (1)
buildCommonLocators(3-151)
packages/bruno-app/src/components/RequestPane/Assertions/AssertionOperator/index.js (3)
packages/bruno-app/src/components/RequestPane/Assertions/index.js (1)
operator(38-38)packages/bruno-js/src/runtime/assert-runtime.js (1)
operator(127-127)packages/bruno-app/src/components/RequestPane/Assertions/AssertionRow/index.js (1)
operator(93-93)
⏰ 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 - macOS
- GitHub Check: Unit Tests
- GitHub Check: SSL Tests - Windows
- GitHub Check: CLI Tests
- GitHub Check: SSL Tests - Linux
🔇 Additional comments (7)
packages/bruno-app/src/components/EditableTable/index.js (1)
19-20: LGTM! Excellent testability improvements.The addition of the
testIdprop and strategic placement ofdata-testidattributes throughout the component enables robust e2e testing without affecting functionality. The default value and consistent naming pattern are well-designed.Also applies to: 228-228, 289-289, 297-297, 304-307
packages/bruno-app/src/components/RequestPane/Assertions/AssertionOperator/index.js (1)
84-84: LGTM! Test ID added for testability.The
data-testidattribute enables stable e2e test selectors without affecting functionality.packages/bruno-schema/src/collections/index.js (2)
36-74: Good schema addition for assertion validation.The
assertionOperatorsconstant andassertionSchemaproperly constrain assertion operator values and extend the existingkeyValueSchema. This adds necessary validation that was previously missing.
415-415: LGTM! Assertions now use the new schema.Updating
requestSchema,grpcRequestSchema, andwsRequestSchemato useassertionSchemainstead ofkeyValueSchemaenables proper operator validation across all request types.Also applies to: 451-451, 489-489
tests/asserts/fixtures/collection/ping.bru (1)
1-11: LGTM! Simple and focused test fixture.The ping request fixture is well-suited for testing assertions with predictable responses. Proper placement in
tests/asserts/fixtures/collection/follows the established pattern for test-specific fixtures.Based on learnings: test-specific collections belong in
tests/**/fixtures/collection.tests/asserts/add-assertions.spec.ts (2)
16-61: Excellent test structure with comprehensive cleanup.The test suite properly uses
test.stepfor readability, has well-organized hooks, and thorough cleanup inafterEach. The cleanup logic correctly handles index shifting when deleting rows.Based on coding guidelines: "Promote the use of
test.stepas much as possible so the generated reports are easier to read."
63-278: LGTM! Comprehensive test coverage for assertion operations.The six test cases thoroughly cover:
- Adding single and multiple assertions
- Editing existing assertions
- Toggle enable/disable functionality
- Deletion with proper state verification
- Various operator types (contains, gt, length)
All tests use proper assertions and state-based waits, with past
waitForTimeoutissues already addressed per the commit history.
Description
This PR fixes the issue were we are unable to add any new assertions to a request
Jira
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
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.