Skip to content

Conversation

@sanjaikumar-bruno
Copy link
Contributor

@sanjaikumar-bruno sanjaikumar-bruno commented Dec 4, 2025

BRU-2258
Fixes: #6290

Summary by CodeRabbit

  • Bug Fixes
    • Improved phone number format consistency in test data generation.
    • Enhanced company suffix generation to use realistic business suffixes for more authentic test names.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 4, 2025

Walkthrough

Refactored faker utility functions to generate predictable outputs: phone numbers now follow a formatted XX-XXX-XXX-XXXX pattern, and company suffixes are selected from a predefined list instead of using generic faker methods. Test patterns updated to match.

Changes

Cohort / File(s) Summary
Faker utility implementation
packages/bruno-common/src/utils/faker-functions.ts
Modified randomPhoneNumberExt() to generate formatted phone numbers (XX-XXX-XXX-XXXX) instead of calling faker.phone.number(); replaced randomCompanySuffix() to select from a fixed list of 9 common suffixes (Inc, LLC, Ltd, Corp, Co, Group, Holdings, Partners, Sons) via faker.helpers.arrayElement instead of faker.company.name()
Faker utility tests
packages/bruno-common/src/utils/faker-functions.spec.ts
Updated regex patterns for randomPhoneNumberExt test from generic ^[\s\S]+$ to specific format \d{2}-\d{3}-\d{3}-\d{4}; updated randomCompanySuffix test regex from generic pattern to constrained set: Inc|LLC|Ltd|Corp|Co|Group|Holdings|Partners|and Sons

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Changes are consistent across both files (implementation and corresponding tests)
  • Logic is straightforward: replacing generic faker calls with deterministic formats
  • Verify that the new phone number format and suffix list meet functional requirements

Poem

📞 From random chaos, order flows,
Phone numbers now in tidy rows,
Suffixes chosen from the shelf,
Predictable, and true to self! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main changes: updating regex patterns for phone number and company suffix validation, and improving the underlying faker functions to use more constrained, realistic data generation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
packages/bruno-common/src/utils/faker-functions.ts (2)

34-34: randomPhoneNumberExt format matches tests; consider template literal for readability

The new implementation correctly matches the ^\d{2}-\d{3}-\d{3}-\d{4}$ pattern. To make it a bit easier to read and maintain, you could switch to a template literal:

-  randomPhoneNumberExt: () => faker.string.numeric(2) + '-' + faker.string.numeric(3) + '-' + faker.string.numeric(3) + '-' + faker.string.numeric(4),
+  randomPhoneNumberExt: () => `${faker.string.numeric(2)}-${faker.string.numeric(3)}-${faker.string.numeric(3)}-${faker.string.numeric(4)}`,

68-68: Deterministic company suffix list looks good; watch for drift with test regex

Using faker.helpers.arrayElement over an explicit suffix list is a nice tightening of behavior and keeps outputs predictable. Since the exact same set of suffixes is also hard-coded in the spec regex, there’s a small risk of the two drifting over time.

If this list evolves further, consider centralizing it (e.g., a shared companySuffixes constant) and deriving both the function and the test regex from that list to keep them in sync, as long as it doesn’t add more complexity than it removes. Based on learnings, this would align with the preference for consistent shared helpers when they improve clarity.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 109394c and e3b25f1.

📒 Files selected for processing (2)
  • packages/bruno-common/src/utils/faker-functions.spec.ts (2 hunks)
  • packages/bruno-common/src/utils/faker-functions.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, never tabs
Use single quotes for strings instead of double quotes
Always add semicolons at the end of statements
No trailing commas in code
Always use parentheses around parameters in arrow functions, even for single parameters
For multiline constructs, put opening braces on the same line with a minimum of 2 elements for multiline formatting
No newlines inside function parentheses
Space before and after the arrow in arrow functions: () => {}
No space between function name and parentheses: func() not func ()
Semicolons should go at the end of the line, not on a new line
Function names should be concise and descriptive
Add JSDoc comments to provide details to abstractions
Avoid single-line abstractions where all that is being done is increasing the call stack with one additional function
Add meaningful comments to explain complex code flow instead of obvious comments

Files:

  • packages/bruno-common/src/utils/faker-functions.ts
  • packages/bruno-common/src/utils/faker-functions.spec.ts
**/*.{test,spec}.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (CODING_STANDARDS.md)

Add tests for any new functionality or meaningful changes; update corresponding tests when code is added, removed, or significantly modified

Files:

  • packages/bruno-common/src/utils/faker-functions.spec.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-03T08:09:57.124Z
Learning: Use consistent patterns and helper utilities where they improve clarity; prefer shared test utilities over copy-pasted setup code only when it reduces complexity
📚 Learning: 2025-12-03T08:09:57.124Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-03T08:09:57.124Z
Learning: Applies to **/*.{test,spec}.{js,jsx,ts,tsx} : Add tests for any new functionality or meaningful changes; update corresponding tests when code is added, removed, or significantly modified

Applied to files:

  • packages/bruno-common/src/utils/faker-functions.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: Playwright E2E Tests
  • GitHub Check: CLI Tests
  • GitHub Check: Unit Tests
  • GitHub Check: SSL Tests - macOS
  • GitHub Check: SSL Tests - Windows
  • GitHub Check: SSL Tests - Linux
🔇 Additional comments (2)
packages/bruno-common/src/utils/faker-functions.spec.ts (2)

51-51: Phone extension regex correctly matches new generator format

The updated randomPhoneNumberExt regex (^\d{2}-\d{3}-\d{3}-\d{4}$) matches the new implementation and keeps the expectation clear and strict. Looks good.


85-85: Company suffix regex correctly constrained to fixed set

The new randomCompanySuffix pattern is now explicitly limited to the same set of suffixes used in the implementation, which is a solid improvement in test precision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

{{$randomCompanySuffix}} Returning Company Name Instead of Suffix

1 participant