-
Notifications
You must be signed in to change notification settings - Fork 8.3k
test: Extract code block detection into shared utilities #11222
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis change introduces dedicated utility functions ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #11222 +/- ##
=======================================
Coverage 33.69% 33.70%
=======================================
Files 1399 1400 +1
Lines 66301 66309 +8
Branches 9788 9785 -3
=======================================
+ Hits 22341 22350 +9
+ Misses 42820 42819 -1
Partials 1140 1140
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/frontend/tests/extended/regression/generalBugs-shard-3.spec.ts (1)
14-21: Environment variable may not be loaded when skip check runs.The
test.skipcheck at line 15 evaluatesprocess.env.OPENAI_API_KEYbeforedotenv.config()is called at line 20. In non-CI local environments, this could cause the test to skip even when the key exists in.env.Consider moving
dotenv.config()before the skip check:Proposed fix
+ if (!process.env.CI) { + dotenv.config({ path: path.resolve(__dirname, "../../.env") }); + } + test.skip( !process?.env?.OPENAI_API_KEY, "OPENAI_API_KEY required to run this test", ); - - if (!process.env.CI) { - dotenv.config({ path: path.resolve(__dirname, "../../.env") }); - }
🧹 Nitpick comments (1)
src/frontend/src/components/core/chatComponents/ContentDisplay.tsx (1)
175-189: Variable shadowing:contentreused in nested scope.The variable
contentat line 176 shadows the outercontentparameter (of typeContentType). While this works correctly at runtime, it reduces readability and could cause confusion during maintenance.Consider renaming to
codeContentor similar:Suggested improvement
code: ({ node, className, children, ...props }) => { - const content = String(children); - if (isCodeBlock(className, props, content)) { + const codeContent = String(children); + if (isCodeBlock(className, props, codeContent)) { return ( <SimplifiedCodeTabComponent language={extractLanguage(className)} - code={content.replace(/\n$/, "")} + code={codeContent.replace(/\n$/, "")} /> ); }
Adam-Aghili
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This pull request refactors how code blocks are detected and rendered in markdown content across the frontend, introducing utility functions for consistent logic and improving maintainability. It also adds comprehensive unit tests for these utilities. The main changes are as follows:
Code block detection and rendering improvements:
isCodeBlockandextractLanguageincodeBlockUtils.tsto standardize detection of code blocks and extraction of language identifiers.ContentDisplay.tsx,edit-message.tsx, andcontent-view.tsxwith calls to the new utility functions, simplifying the code and ensuring consistent behavior across components. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]Testing:
isCodeBlockandextractLanguageincodeBlockUtils.test.ts, covering various edge cases and ensuring robust behavior.Other:
Summary by CodeRabbit
Release Notes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.