test: add test coverage for TimedConnection#2009
test: add test coverage for TimedConnection#2009Harsh16gupta wants to merge 1 commit intoasyncapi:masterfrom
Conversation
Harsh16gupta
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.
|
📝 WalkthroughWalkthroughA new integration test file for the TimedConnection React component has been added with 53 lines of code. Tests use an AsyncAPI parser and fixture to verify component rendering via snapshots under various 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)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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: 2
🧹 Nitpick comments (1)
packages/templates/clients/websocket/java/quarkus/test/components/TimedConnection.test.js (1)
1-53: Add JSDoc to thebeforeAllcallback at minimum.As per coding guidelines, functions in
.jsfiles should have JSDoc. Thetest(...)names are self-documenting, but thebeforeAllasync callback — which sets up shared state and has failure modes — benefits from a brief@descriptionnoting what it loads and whatparsedAsyncAPIDocument/sendOperationsrepresent.♻️ Suggested JSDoc for `beforeAll`
+ /** + * Parses the shared AsyncAPI fixture and extracts send operations + * used by the fixture-driven test. + * `@async` + */ beforeAll(async () => {As per coding guidelines: "Write clear and consistent JSDoc comments for functions, including parameter types, return values, and error conditions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/templates/clients/websocket/java/quarkus/test/components/TimedConnection.test.js` around lines 1 - 53, Add a JSDoc block above the beforeAll async callback describing its purpose and what it initializes (parsedAsyncAPIDocument and sendOperations), including a brief `@description`, any relevant return/throws note (it’s async and may reject), and types for the variables (e.g., parsedAsyncAPIDocument: AsyncAPIDocument, sendOperations: Array). Locate the beforeAll function in the TimedConnection.test.js file and insert a short JSDoc that documents the setup behavior and the meanings of parsedAsyncAPIDocument and sendOperations so linters and readers understand the shared test state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/templates/clients/websocket/java/quarkus/test/components/TimedConnection.test.js`:
- Around line 13-18: Add a validity guard after parsing in the beforeAll so
parsedAsyncAPIDocument is asserted before calling
parsedAsyncAPIDocument.operations() (e.g., add
expect(parsedAsyncAPIDocument).toBeDefined()); and remove the coupling of the
three fixture-independent tests from this describe that uses beforeAll by
relocating those tests (the ones that call with null/undefined/[]) into a
sibling describe or a nested describe without the beforeAll so failures parsing
in beforeAll do not cascade to tests that don’t use sendOperations (references:
beforeAll, parsedAsyncAPIDocument, sendOperations).
- Line 7: Add JSDoc comments above the beforeAll callback and each test callback
in TimedConnection.test.js: for the beforeAll callback (the async setup
function) add a JSDoc block describing that it initializes test fixtures and is
asynchronous, include `@async` and `@returns` {Promise<void>}; for each test()
callback add a JSDoc block that succinctly states what the test verifies (e.g.,
verifies connection establishment, message timing, or teardown behavior),
include `@async` if the test callback is async and `@returns` {Promise<void>} for
consistency; place the comments immediately above the beforeAll and each
test(...) so linters and coding guidelines recognize them.
---
Nitpick comments:
In
`@packages/templates/clients/websocket/java/quarkus/test/components/TimedConnection.test.js`:
- Around line 1-53: Add a JSDoc block above the beforeAll async callback
describing its purpose and what it initializes (parsedAsyncAPIDocument and
sendOperations), including a brief `@description`, any relevant return/throws note
(it’s async and may reject), and types for the variables (e.g.,
parsedAsyncAPIDocument: AsyncAPIDocument, sendOperations: Array). Locate the
beforeAll function in the TimedConnection.test.js file and insert a short JSDoc
that documents the setup behavior and the meanings of parsedAsyncAPIDocument and
sendOperations so linters and readers understand the shared test state.
| import TimedConnection from '../../components/TimedConnection.js'; | ||
|
|
||
| const parser = new Parser(); | ||
| const asyncapiFilePath = path.resolve(__dirname, '../../../../test/__fixtures__/asyncapi-websocket-components.yml'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1. Search for the fixture file
echo "=== Searching for asyncapi-websocket-components.yml ==="
fd "asyncapi-websocket-components.yml" --type f 2>/dev/null || echo "File not found with fd"
# 2. Look for package.json in the quarkus directory and check it
echo -e "\n=== Checking package.json in quarkus package ==="
if [ -f packages/templates/clients/websocket/java/quarkus/package.json ]; then
cat packages/templates/clients/websocket/java/quarkus/package.json
else
echo "Not found at expected location"
fi
# 3. Check parent package.json files for module configuration
echo -e "\n=== Checking parent package.json files ==="
for pkgjson in packages/package.json packages/templates/package.json packages/templates/clients/package.json packages/templates/clients/websocket/package.json; do
if [ -f "$pkgjson" ]; then
echo "--- $pkgjson ---"
cat "$pkgjson" | head -20
fi
done
# 4. Check how other test files in the package handle __dirname
echo -e "\n=== Other test files in the quarkus package ==="
fd "\.test\.js$" packages/templates/clients/websocket/java/quarkus/ --type fRepository: asyncapi/generator
Length of output: 2895
🏁 Script executed:
#!/bin/bash
# Check a couple of other test files to see if they use __dirname in the same way
echo "=== Checking ClientConnector.test.js ==="
head -20 packages/templates/clients/websocket/java/quarkus/test/components/ClientConnector.test.js
echo -e "\n=== Checking EchoWebSocket.test.js ==="
head -20 packages/templates/clients/websocket/java/quarkus/test/components/EchoWebSocket.test.js
# Also check the full TimedConnection.test.js to see the JSDoc situation
echo -e "\n=== Full TimedConnection.test.js ==="
cat -n packages/templates/clients/websocket/java/quarkus/test/components/TimedConnection.test.js
# Check if there's a jest.config.js or similar
echo -e "\n=== Jest configuration ==="
fd "jest.config" packages/templates/clients/websocket/java/quarkus/ --type f
if [ -f packages/templates/clients/websocket/java/quarkus/jest.config.js ]; then
cat packages/templates/clients/websocket/java/quarkus/jest.config.js
fiRepository: asyncapi/generator
Length of output: 4085
Add JSDoc comments for test functions and beforeAll callback to comply with coding guidelines.
The fixture path is correct—the file exists at packages/templates/clients/websocket/test/__fixtures__/asyncapi-websocket-components.yml and the path resolution from test/components/ is accurate. __dirname is also available: this project uses Babel transformation via babel-jest (configured in jest.config.js), which converts ES modules to CommonJS at runtime, making __dirname accessible. Other test files in the same directory (ClientConnector.test.js, EchoWebSocket.test.js) already use this identical pattern successfully.
However, the file lacks JSDoc comments for functions as required by the coding guidelines. Add JSDoc to:
- The
beforeAllcallback (lines 13–18): describe what it does, that it's async, return type - Each
test()callback (lines 20–51): describe what each test verifies
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/templates/clients/websocket/java/quarkus/test/components/TimedConnection.test.js`
at line 7, Add JSDoc comments above the beforeAll callback and each test
callback in TimedConnection.test.js: for the beforeAll callback (the async setup
function) add a JSDoc block describing that it initializes test fixtures and is
asynchronous, include `@async` and `@returns` {Promise<void>}; for each test()
callback add a JSDoc block that succinctly states what the test verifies (e.g.,
verifies connection establishment, message timing, or teardown behavior),
include `@async` if the test callback is async and `@returns` {Promise<void>} for
consistency; place the comments immediately above the beforeAll and each
test(...) so linters and coding guidelines recognize them.
| beforeAll(async () => { | ||
| const parseResult = await fromFile(parser, asyncapiFilePath).parse(); | ||
| parsedAsyncAPIDocument = parseResult.document; | ||
| const operations = parsedAsyncAPIDocument.operations(); | ||
| sendOperations = operations.filterBySend(); | ||
| }); |
There was a problem hiding this comment.
beforeAll failure cascades to fixture-independent tests; add a document guard.
Two issues in this block:
-
Missing validity guard:
parseResult.documentisundefinedwhen the AsyncAPI document is invalid. Calling.operations()on it will throw aTypeError, making the error misleading. Add a guard:expect(parsedAsyncAPIDocument).toBeDefined();
-
Unnecessary coupling: Tests 1–3 pass
null,undefined, and[]directly and never readsendOperations. Because they share thisdescribe/beforeAll, a fixture-load failure will fail them too, masking what actually broke. Consider moving them into a siblingdescribeblock without abeforeAll, or into a nesteddescribethat skips setup.
♻️ Proposed restructuring
-describe('TimedConnection component (integration with AsyncAPI document)', () => {
- let parsedAsyncAPIDocument;
- let sendOperations;
-
- beforeAll(async () => {
- const parseResult = await fromFile(parser, asyncapiFilePath).parse();
- parsedAsyncAPIDocument = parseResult.document;
- const operations = parsedAsyncAPIDocument.operations();
- sendOperations = operations.filterBySend();
- });
-
- test('renders generic message when sendOperations is null', () => { ... });
- test('renders generic message when sendOperations is undefined', () => { ... });
- test('renders generic message when sendOperations is empty array', () => { ... });
- test('renders specific message with operationId when sendOperations has data from fixture', () => { ... });
-});
+describe('TimedConnection component', () => {
+ describe('without fixture — edge cases', () => {
+ test('renders generic message when sendOperations is null', () => { ... });
+ test('renders generic message when sendOperations is undefined', () => { ... });
+ test('renders generic message when sendOperations is empty array', () => { ... });
+ });
+
+ describe('integration with AsyncAPI document', () => {
+ let parsedAsyncAPIDocument;
+ let sendOperations;
+
+ beforeAll(async () => {
+ const parseResult = await fromFile(parser, asyncapiFilePath).parse();
+ parsedAsyncAPIDocument = parseResult.document;
+ expect(parsedAsyncAPIDocument).toBeDefined();
+ sendOperations = parsedAsyncAPIDocument.operations().filterBySend();
+ });
+
+ test('renders specific message with operationId when sendOperations has data from fixture', () => { ... });
+ });
+});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/templates/clients/websocket/java/quarkus/test/components/TimedConnection.test.js`
around lines 13 - 18, Add a validity guard after parsing in the beforeAll so
parsedAsyncAPIDocument is asserted before calling
parsedAsyncAPIDocument.operations() (e.g., add
expect(parsedAsyncAPIDocument).toBeDefined()); and remove the coupling of the
three fixture-independent tests from this describe that uses beforeAll by
relocating those tests (the ones that call with null/undefined/[]) into a
sibling describe or a nested describe without the beforeAll so failures parsing
in beforeAll do not cascade to tests that don’t use sendOperations (references:
beforeAll, parsedAsyncAPIDocument, sendOperations).
2 participants
Description
Adds comprehensive integration tests for the
TimedConnectioncomponent in the Java Quarkus WebSocket client template.Fixes #1937
Summary by CodeRabbit