test: add tests for Installation README component#1945
test: add tests for Installation README component#1945asyncapi-bot merged 6 commits intoasyncapi:masterfrom
Conversation
SHUBHANSHU602
requested review from
Adi-204,
asyncapi-bot-eve,
derberg and
magicmatatjahu
as code owners
|
What reviewer looks at during PR reviewThe following are ideal points maintainers look for during review. Reviewing these points yourself beforehand can help streamline the review process and reduce time to merge.
|
📝 WalkthroughWalkthroughAdds a new test suite file for the Installation component containing three tests: snapshot tests for JavaScript and Python installation commands, and a test for undefined Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/components/test/components/Installation.test.js`:
- Around line 15-18: The test for the Installation component currently snapshots
output when language is undefined but doesn't assert intent; update the test to
explicitly assert expected behavior: in the Installation.test.js modify the test
for Rendering when language is undefined to add either
expect(result).not.toContain('undefined') if undefined should be rejected, or
expect(result).toContain('undefined') if that behavior is intentional, and
optionally adjust the component by adding a safe fallback in installCommands
lookup (e.g., use installCommands[language] ?? 'unknown' inside the Installation
component) so the test and component behavior match; locate references to
Installation, installCommands and the language prop to make the change.
🧹 Nitpick comments (2)
packages/components/test/components/Installation.test.js (2)
5-13: Consider adding at least one inline assertion alongside snapshots.Snapshot tests are great for regression detection but don't communicate intent. A complementary assertion (e.g.,
expect(result).toContain('npm install')for JS,expect(result).toContain('pip install')for Python) would make failures self-explanatory and guard against snapshot updates that accidentally accept wrong output.Example complementary assertions
test('renders javascript installation command', () => { const result = render(<Installation language="javascript" />); + expect(result).toContain('npm install'); expect(result.trim()).toMatchSnapshot(); }); test('renders python installation command', () => { const result = render(<Installation language="python" />); + expect(result).toContain('pip install'); expect(result.trim()).toMatchSnapshot(); });
4-19: Missing JSDoc comments on test suite.Per the coding guidelines for
**/*.{js,ts,jsx,tsx}, functions should include JSDoc comments. A brief top-level doc block describing what this suite covers would satisfy the guideline. As per coding guidelines, "Write clear and consistent JSDoc comments for functions, including parameter types, return values, and error conditions."
|
|
||
| test('renders installation section when language is undefined', () => { | ||
| const result = render(<Installation />); | ||
| expect(result.trim()).toMatchSnapshot(); |
There was a problem hiding this comment.
| expect(result.trim()).toMatchSnapshot(); | |
| expect(actual).toBe(''); |
Until we don't have proper error handling no need to match snapshot.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/components/test/components/Installation.test.js`:
- Line 19: Update the indentation of the inline comment "// Explicitly document
current behavior" in the Installation.test.js snippet from 2 spaces to 4 spaces
to satisfy the project's ESLint rules; locate the comment in the test file (the
line containing that exact comment) and replace the leading two spaces with four
so the file passes the pipeline.
There was a problem hiding this comment.
@SHUBHANSHU602 did you read point 2 Note in the comment - #1945 (comment)
hey @Adi-204 if you talking about the issue number , I added it |
|
|
/rtm |
3 participants
This PR adds unit test coverage for the README Installation component, which is a public-facing component rendered in generated documentation.
The component contains language-dependent logic (python vs javascript) but previously had no direct tests validating the rendered output.
Resolves #1904
Summary by CodeRabbit