test(keeper): add utils.test.js for parseAsyncAPIDocumentFromFile#1959
test(keeper): add utils.test.js for parseAsyncAPIDocumentFromFile#1959moksha-hub wants to merge 2 commits intoasyncapi:masterfrom
Conversation
Adds comprehensive test coverage for the parseAsyncAPIDocumentFromFile utility function: - Input validation tests for null, undefined, empty string, whitespace, number, object, and array inputs - Error wrapping tests to verify parser errors are properly wrapped - Happy path tests for successful document parsing Closes asyncapi#1955
moksha-hub
requested review from
asyncapi-bot-eve,
derberg,
jonaslagoni and
magicmatatjahu
as code owners
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
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 for parseAsyncAPIDocumentFromFile that verifies input validation for invalid asyncapiFilepath values, ensures parser errors are wrapped with a standardized message preserving original details, and asserts successful parsing of a valid AsyncAPI file. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. 🎉 🧹 Recent nitpick comments
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 `@apps/keeper/test/utils.test.js`:
- Around line 59-66: The test "should include file-related error details in
wrapped message" uses the removed Jasmine global fail() and duplicates an
earlier assertion; either delete this redundant test or rewrite it to use Jest's
rejects pattern and assert file details: replace the try/catch + fail usage with
await
expect(parseAsyncAPIDocumentFromFile(invalidAsyncapiPath)).rejects.toThrow(expect.stringContaining('Failed
to parse AsyncAPI document')) and add an assertion that the error message
contains invalidAsyncapiPath (or other file-related details) to match the test
description; reference parseAsyncAPIDocumentFromFile and the test name when
locating the code to change.
🧹 Nitpick comments (1)
apps/keeper/test/utils.test.js (1)
8-50: Consider usingtest.eachto reduce repetition.All seven input validation tests share the same assertion logic. A parameterized table would be more concise and easier to extend.
♻️ Suggested refactor
- describe('Input Validation', () => { - test('should throw error if asyncapiFilepath is null', async () => { - await expect(parseAsyncAPIDocumentFromFile(null)).rejects.toThrow( - 'Invalid "asyncapiFilepath" parameter: must be a non-empty string' - ); - }); - - test('should throw error if asyncapiFilepath is undefined', async () => { - await expect(parseAsyncAPIDocumentFromFile(undefined)).rejects.toThrow( - 'Invalid "asyncapiFilepath" parameter: must be a non-empty string' - ); - }); - - test('should throw error if asyncapiFilepath is empty string', async () => { - await expect(parseAsyncAPIDocumentFromFile('')).rejects.toThrow( - 'Invalid "asyncapiFilepath" parameter: must be a non-empty string' - ); - }); - - test('should throw error if asyncapiFilepath is whitespace only', async () => { - await expect(parseAsyncAPIDocumentFromFile(' ')).rejects.toThrow( - 'Invalid "asyncapiFilepath" parameter: must be a non-empty string' - ); - }); - - test('should throw error if asyncapiFilepath is a number', async () => { - await expect(parseAsyncAPIDocumentFromFile(123)).rejects.toThrow( - 'Invalid "asyncapiFilepath" parameter: must be a non-empty string' - ); - }); - - test('should throw error if asyncapiFilepath is an object', async () => { - await expect(parseAsyncAPIDocumentFromFile({ path: 'test.yml' })).rejects.toThrow( - 'Invalid "asyncapiFilepath" parameter: must be a non-empty string' - ); - }); - - test('should throw error if asyncapiFilepath is an array', async () => { - await expect(parseAsyncAPIDocumentFromFile(['test.yml'])).rejects.toThrow( - 'Invalid "asyncapiFilepath" parameter: must be a non-empty string' - ); - }); - }); + describe('Input Validation', () => { + test.each([ + ['null', null], + ['undefined', undefined], + ['empty string', ''], + ['whitespace only', ' '], + ['a number', 123], + ['an object', { path: 'test.yml' }], + ['an array', ['test.yml']], + ])('should throw error if asyncapiFilepath is %s', async (_label, input) => { + await expect(parseAsyncAPIDocumentFromFile(input)).rejects.toThrow( + 'Invalid "asyncapiFilepath" parameter: must be a non-empty string' + ); + }); + });
Addresses CodeRabbit review feedback: - Removed fail() which is not available in Jest 27+ - Replaced with rejects.toThrow pattern using expect.objectContaining
|
|
hey @moksha-hub , here in this repo we wait for maintainer to approve the issue first , also I have shown interest in solving the issue so please close you PR and read the contribution guidelines. Thanks! |
|
@SHUBHANSHU602 Thanks for information, I was misunderstood earlier about the guidelines and for now as I opened it and worked on raising the PR I wish not to close, and wait for decision takers or assigners to take any further actions. |
|
Thanks for the contribution. I’m closing this PR because, while there is an existing issue, it was not approved or opened for contribution before the pull request was submitted. Please make sure to follow the contribution guidelines, which explain when issues are considered ready for implementation and when pull requests are appropriate |
4 participants
Adds comprehensive test coverage for the parseAsyncAPIDocumentFromFile utility function:
Closes #1955
Description
Related issue(s)
Summary by CodeRabbit