chore: add acceptance testing with microcks mocks #1467
Conversation
|
WalkthroughThe pull request introduces multiple enhancements for WebSocket client testing and messaging. In both JavaScript and Python clients, message-sending methods are updated to support static and asynchronous operations. Several AsyncAPI documents now include additional examples and message definitions, while new acceptance tests, utility scripts, and configuration files have been added to facilitate contract testing with Microcks. The changes also include Podman configuration and a health-check script for the testing environment, along with an update to the Changes
Sequence Diagram(s)sequenceDiagram
participant Tester as Acceptance Test
participant Client as WSClient
participant Server as WebSocket Server
participant Microcks as Microcks API
Tester->>Client: Initialize WebSocket connection
Client->>Server: Send message via static/async method
Server->>Client: Echo message back
Client->>Tester: Deliver received message
Tester->>Microcks: POST test request for contract validation
Microcks-->>Tester: Return test success confirmation
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
derberg
requested review from
Florence-Njeri,
asyncapi-bot-eve,
magicmatatjahu and
jonaslagoni
as code owners
|
derberg
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (23)
packages/templates/clients/websocket/test/python/requirements.txt (1)
1-5: Consider updating certifi version based on previous standards.The dependencies for Python integration testing look good overall. However, based on previous conventions in the project, consider using a forward-looking version for certifi.
From project learnings, it appears that other Python WebSocket templates use certifi with forward-looking versions:
-certifi==2023.11.17 +certifi==2025.1.31This approach ensures up-to-date certificates when deployed, as noted in previous project conventions.
packages/templates/clients/websocket/test/javascript/package.json (1)
1-6: Well-structured package.json and clear script definitions.
Thescriptssection defines testing and linting commands clearly. The relative paths for ESLint configuration are deep; please confirm that these paths work as intended in all environments.packages/templates/clients/websocket/javascript/test/__fixtures__/asyncapi-hoppscotch-echo.yml (1)
16-17: Typographical remark on channel description.
There is a minor typo in the channel description ("timestampl" on line 17). Consider correcting it to "timestamp" for clarity.packages/templates/clients/websocket/test/__fixtures__/asyncapi-hoppscotch-server.yml (2)
14-24: Consistent channel and binding definitions.
Channels, messages, and the binding over WebSocket are well defined. However, note the same typo ("timestampl") in the echo channel description (line 17).
44-71: Attention to component definitions and trailing whitespace.
The components section (messages and schemas) is detailed and specifies examples clearly. Please remove the trailing spaces as flagged by the static analysis (e.g., on lines 31, 38, 48, 64, 66) and ensure a newline at the end of file (line 71).🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 48-48: trailing spaces
(trailing-spaces)
[error] 64-64: trailing spaces
(trailing-spaces)
[error] 66-66: trailing spaces
(trailing-spaces)
[error] 71-71: no new line character at the end of file
(new-line-at-end-of-file)
packages/templates/clients/websocket/test/microcks-setup/config/application.properties (1)
1-28: Well-organized configuration file for Microcks setup.
The file provides clear settings for asynchronous API mocking, Microcks API, Keycloak, and Kafka broker access. The commented-out sections serve as useful reference for future configurations.
Consider removing or further documenting the commented out lines if they are no longer needed to keep the configuration clean.packages/templates/clients/websocket/test/microcks-setup/checkMicrocksReady.sh (1)
9-11: Enhance robustness of health status check.
Consider handling cases wherepodman inspectmight return an empty result. For example, you could modify the condition as follows to also catch an empty status:- if [[ "$status" != "healthy" ]]; then + if [[ -z "$status" || "$status" != "healthy" ]]; thenThis ensures that a failure in retrieving the status doesn’t inadvertently pass the health check.
packages/templates/clients/websocket/test/microcks-setup/config/features.properties (1)
1-8: Configuration looks good but consider environment variable support.The Microcks configuration properly enables async API features and defines endpoints for various protocols. For improved flexibility in different environments, consider using environment variables for the endpoint URLs rather than hardcoded values.
features.feature.async-api.enabled=true features.feature.async-api.frequencies=3,10,30 features.feature.async-api.default-binding=KAFKA -features.feature.async-api.endpoint-KAFKA=localhost:9092 -features.feature.async-api.endpoint-MQTT=my-mqtt-broker.apps.try.microcks.io:1883 -features.feature.async-api.endpoint-AMQP=my-amqp-broker.apps.try.microcks.io:5672 -features.feature.async-api.endpoint-WS=localhost:8081 -features.feature.async-api.endpoint-NATS=localhost:4222 +features.feature.async-api.endpoint-KAFKA=${KAFKA_ENDPOINT:localhost:9092} +features.feature.async-api.endpoint-MQTT=${MQTT_ENDPOINT:my-mqtt-broker.apps.try.microcks.io:1883} +features.feature.async-api.endpoint-AMQP=${AMQP_ENDPOINT:my-amqp-broker.apps.try.microcks.io:5672} +features.feature.async-api.endpoint-WS=${WS_ENDPOINT:localhost:8081} +features.feature.async-api.endpoint-NATS=${NATS_ENDPOINT:localhost:4222}packages/templates/clients/websocket/test/README.md (5)
15-15: Fix grammar and improve clarity.There are a few grammatical issues in this sentence.
-You need to remember about `--profile test` to run whole setup with tests. This way you ensure that proper importer container imports `__fixtures__/asyncapi-hoppscotch-server.yml` into Microcks and tests run against it. +You need to remember about `--profile tests` to run the whole setup with tests. This way, you ensure that the proper importer container imports `__fixtures__/asyncapi-hoppscotch-server.yml` into Microcks and tests run against it.🧰 Tools
🪛 LanguageTool
[uncategorized] ~15-~15: You might be missing the article “the” here.
Context: ... remember about--profile testto run whole setup with tests. This way you ensure t...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~15-~15: A comma might be missing here.
Context: ...st` to run whole setup with tests. This way you ensure that proper importer contain...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
21-21: Fix grammar and word order.The sentence has some grammatical issues that affect readability.
-Tests also run in containers, although with provided configuration you can start also testing environment without running tests, just to play with Microcks and understand more how it works. +Tests also run in containers, although with the provided configuration, you can also start the testing environment without running tests, just to play with Microcks and understand more about how it works.🧰 Tools
🪛 LanguageTool
[uncategorized] ~21-~21: You might be missing the article “the” here.
Context: ...s also run in containers, although with provided configuration you can start also testin...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[formatting] ~21-~21: Consider inserting a comma here, unless the first half is essential to the meaning of the sentence.
Context: ...n in containers, although with provided configuration you can start also testing environment ...(WITH_THAT_COMMA)
[style] ~21-~21: To make your writing flow more naturally, try moving ‘also’ before the verb.
Context: ...lthough with provided configuration you can start also testing environment without running tes...(ALSO_PLACEMENT)
29-29: Add missing article.-This instruction assumes you are located in directory where `microcks-setup` directory is located +This instruction assumes you are located in the directory where `microcks-setup` directory is located🧰 Tools
🪛 LanguageTool
[uncategorized] ~29-~29: You might be missing the article “the” here.
Context: ... instruction assumes you are located in directory wheremicrocks-setupdirectory is loc...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
35-35: Fix grammatical issues.-Check with `podman ps` command if all services are running. It may take few minutes to start all containers. You can also run special script that will confirm services are ready: `bash ./microcks-setup/checkMicrocksReady.sh`. +Check with the `podman ps` command if all services are running. It may take a few minutes to start all containers. You can also run a special script that will confirm services are ready: `bash ./microcks-setup/checkMicrocksReady.sh`.🧰 Tools
🪛 LanguageTool
[uncategorized] ~35-~35: You might be missing the article “a” here.
Context: ...f all services are running. It may take few minutes to start all containers. You ca...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~35-~35: You might be missing the article “the” here.
Context: ...g. It may take few minutes to start all containers. You can also run special script that w...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~35-~35: You might be missing the article “a” here.
Context: ... start all containers. You can also run special script that will confirm services are r...(AI_EN_LECTOR_MISSING_DETERMINER_A)
89-90: Add language specifier to fenced code block.According to Markdown best practices, fenced code blocks should have a language specified.
-``` +```bash websocat ws://localhost:8081/api/ws/Hoppscotch+WebSocket+Server/1.0.0/sendTimeStampMessage🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
89-89: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
packages/templates/clients/websocket/test/javascript/acceptance.test.js (4)
22-22: Expected message pattern is fragile.The test expects a very specific timestamp format which could make the test brittle. Consider using a more flexible pattern matching approach.
- const expectedMessage = '11:13:24 GMT+0000 (Coordinated Universal Time)'; + // Use regex or partial matching instead of exact time + const expectedTimePattern = /\d{2}:\d{2}:\d{2} GMT/;Then modify the check at line 41-43:
- const isReceived = received_messages.some((message) => - message.includes(expectedMessage) - ); + const isReceived = received_messages.some((message) => + expectedTimePattern.test(message) + );
75-75: Remove commented debugging code.There's a commented line with a long delay that appears to be debugging code.
- //await delay(100000);
11-56: Client receiver test looks good, but consider improving error handling.The test flow is logically sound and well-structured. To make it more robust, consider adding error handling and timeout messages to provide better debugging information.
Add a try-catch block and better error output when expected messages aren't received:
it('javascript client should receive a message', async () => { const received_messages = []; const expectedMessage = '11:13:24 GMT+0000 (Coordinated Universal Time)'; /* * TEST: we test if generated message handler can be registered and later records messages */ // registering message handler that adds incomming messages to an array // later messages from array are evaluated to make sure the message sent from server is received by the client wsClient.registerMessageHandler((message) => { received_messages.push(message); }); /* * TEST: we test if we can connect to server using genrated client */ + try { await wsClient.connect(); // wait for the incomming message to be logged await waitForMessage(received_messages, expectedMessage); const isReceived = received_messages.some((message) => message.includes(expectedMessage) ); // checking if microcks mock send proper message and they were recorded by registered handler for incomming messages expect(isReceived).toEqual(true); + } catch (error) { + console.error('Test failed with error:', error); + console.log('Received messages:', received_messages); + throw error; + } });
58-97: Sender test implementation is correct but contains hardcoded values.The sender test implementation is logically sound. However, several hardcoded values could be moved to constants at the top of the file to improve maintainability.
Consider extracting hardcoded values to constants at the top of the file:
const WSClient = require('../../javascript/test/temp/snapshotTestResult/client-hoppscotch'); const { waitForMessage, delay, waitForTestSuccess } = require('./utils'); const fetch = require('node-fetch'); const WebSocket = require('ws'); -const microcksTestEndpoint = 'http://microcks:8080/api/tests'; +// Configuration constants +const MICROCKS_HOST = 'microcks'; +const MICROCKS_PORT = 8080; +const MICROCKS_TEST_ENDPOINT = `http://${MICROCKS_HOST}:${MICROCKS_PORT}/api/tests`; +const SENDER_TEST_PORT = 8082; +const SENDER_TEST_PATH = '/ws'; +const SERVICE_ID = 'Hoppscotch WebSocket Server:1.0.0'; +const TEST_TIMEOUT = 30000;Then use these constants throughout the test.
packages/templates/clients/websocket/test/python/test_acceptance.py (1)
63-65: Fix typo in commentThere's a typo in the comment: "improtant" should be "important".
- # Most improtant part of test where we test clients send message + # Most important part of test where we test clients send messagepackages/templates/clients/websocket/test/microcks-setup/microcks-podman.yml (3)
3-3: Fix YAML formatting issuesThere are trailing spaces on multiple lines and the file is missing a newline at the end.
-volumes: - microcks-data: - +volumes: + microcks-data: + ... - retries: 3 - + retries: 3 + ... - command: ["sh", "-c", "npm install && cd packages/templates/clients/websocket/test/javascript && npm test"] - + command: ["sh", "-c", "npm install && cd packages/templates/clients/websocket/test/javascript && npm test"] + ... - # Below for debugging purposes. + # Below for debugging purposes. ... - # sh -c "apk add --no-cache websocat && sh" + # sh -c "apk add --no-cache websocat && sh" +Also applies to: 92-92, 126-126, 142-142, 151-151
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 3-3: trailing spaces
(trailing-spaces)
110-111: Convert TODOs to tracked issuesThe TODOs should be tracked in your issue management system rather than remaining as comments in the code.
- #TODO:1: make these tester containers work in way that if test fails, they fails as well. So also good to do starting of podman in a way that it is awaiting status - #TODO:2: add CI/CD pipeline for these tests + # See issue #XXXX: make these tester containers work in way that if test fails, they fails as well + # See issue #YYYY: add CI/CD pipeline for these testsWould you like me to help draft the issue descriptions for these TODOs?
140-140: Simplify long command by using a startup scriptThe command for the Python tester is quite long and hard to read. Consider moving it to a dedicated script file.
Create a
run_python_tests.shscript in the test directory with the command content, and then reference it from the YAML file:- command: ["sh", "-c", "cd packages/templates/clients/websocket/python/test/temp/snapshotTestResult && pip install -r requirements.txt && cd ../../../../test/python && pip install -r requirements.txt && pytest"] + command: ["sh", "-c", "packages/templates/clients/websocket/test/run_python_tests.sh"]packages/templates/clients/websocket/test/javascript/utils.js (2)
56-56: Remove trailing whitespaceThere are trailing spaces on lines 56 and 65.
} - + ... } - +Also applies to: 65-65
46-55: Add more descriptive error message for waitForMessageThe error message when a message isn't found could be more informative by including the actual messages that were received.
async function waitForMessage(a, expectedMessage, timeout = 3000, interval = 10) { const start = Date.now(); while (Date.now() - start < timeout) { if (a.some(message => message.includes(expectedMessage))) { return; } await delay(interval); } - throw new Error(`Expected message "${expectedMessage}" not found within timeout`); + throw new Error(`Expected message "${expectedMessage}" not found within timeout. Received messages: ${JSON.stringify(a)}`); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
package-lock.jsonis excluded by!**/package-lock.jsonpackages/templates/clients/websocket/javascript/test/__snapshots__/integration.test.js.snapis excluded by!**/*.snappackages/templates/clients/websocket/python/test/__snapshots__/integration.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (16)
.gitignore(1 hunks)packages/templates/clients/websocket/javascript/components/SendEchoMessage.js(1 hunks)packages/templates/clients/websocket/javascript/test/__fixtures__/asyncapi-hoppscotch-echo.yml(1 hunks)packages/templates/clients/websocket/javascript/test/__fixtures__/asyncapi-postman-echo.yml(1 hunks)packages/templates/clients/websocket/python/components/SendEchoMessage.js(1 hunks)packages/templates/clients/websocket/test/README.md(1 hunks)packages/templates/clients/websocket/test/__fixtures__/asyncapi-hoppscotch-server.yml(1 hunks)packages/templates/clients/websocket/test/javascript/acceptance.test.js(1 hunks)packages/templates/clients/websocket/test/javascript/package.json(1 hunks)packages/templates/clients/websocket/test/javascript/utils.js(1 hunks)packages/templates/clients/websocket/test/microcks-setup/checkMicrocksReady.sh(1 hunks)packages/templates/clients/websocket/test/microcks-setup/config/application.properties(1 hunks)packages/templates/clients/websocket/test/microcks-setup/config/features.properties(1 hunks)packages/templates/clients/websocket/test/microcks-setup/microcks-podman.yml(1 hunks)packages/templates/clients/websocket/test/python/requirements.txt(1 hunks)packages/templates/clients/websocket/test/python/test_acceptance.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
packages/templates/clients/websocket/test/python/requirements.txt (1)
Learnt from: derberg
PR: asyncapi/generator#1416
File: packages/templates/clients/python/websocket/template/requirements.txt.js:8-8
Timestamp: 2025-04-10T08:04:22.371Z
Learning: The certifi Python package uses date-based versioning (YYYY.MM.DD format). In the python websocket client template, a forward-looking version (2025.1.31) is intentionally used to ensure up-to-date certificates when deployed.
🧬 Code Graph Analysis (2)
packages/templates/clients/websocket/test/javascript/acceptance.test.js (2)
packages/templates/clients/websocket/test/javascript/utils.js (2)
fetch(2-2)response(13-16)packages/templates/clients/websocket/javascript/example.js (1)
message(26-26)
packages/templates/clients/websocket/test/javascript/utils.js (2)
packages/templates/clients/websocket/test/javascript/acceptance.test.js (2)
fetch(7-7)require(6-6)packages/templates/clients/websocket/javascript/example.js (2)
interval(25-25)message(26-26)
🪛 LanguageTool
packages/templates/clients/websocket/test/README.md
[uncategorized] ~15-~15: You might be missing the article “the” here.
Context: ... remember about --profile test to run whole setup with tests. This way you ensure t...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~15-~15: A comma might be missing here.
Context: ...st` to run whole setup with tests. This way you ensure that proper importer contain...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~21-~21: You might be missing the article “the” here.
Context: ...s also run in containers, although with provided configuration you can start also testin...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[formatting] ~21-~21: Consider inserting a comma here, unless the first half is essential to the meaning of the sentence.
Context: ...n in containers, although with provided configuration you can start also testing environment ...
(WITH_THAT_COMMA)
[style] ~21-~21: To make your writing flow more naturally, try moving ‘also’ before the verb.
Context: ...lthough with provided configuration you can start also testing environment without running tes...
(ALSO_PLACEMENT)
[uncategorized] ~29-~29: You might be missing the article “the” here.
Context: ... instruction assumes you are located in directory where microcks-setup directory is loc...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~35-~35: You might be missing the article “a” here.
Context: ...f all services are running. It may take few minutes to start all containers. You ca...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~35-~35: You might be missing the article “the” here.
Context: ...g. It may take few minutes to start all containers. You can also run special script that w...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~35-~35: You might be missing the article “a” here.
Context: ... start all containers. You can also run special script that will confirm services are r...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
🪛 Gitleaks (8.21.2)
packages/templates/clients/websocket/test/README.md
50-50: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
68-68: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
packages/templates/clients/websocket/test/microcks-setup/microcks-podman.yml
108-108: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 markdownlint-cli2 (0.17.2)
packages/templates/clients/websocket/test/README.md
89-89: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🪛 YAMLlint (1.35.1)
packages/templates/clients/websocket/test/__fixtures__/asyncapi-hoppscotch-server.yml
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 38-38: trailing spaces
(trailing-spaces)
[error] 48-48: trailing spaces
(trailing-spaces)
[error] 64-64: trailing spaces
(trailing-spaces)
[error] 66-66: trailing spaces
(trailing-spaces)
[error] 71-71: no new line character at the end of file
(new-line-at-end-of-file)
packages/templates/clients/websocket/test/microcks-setup/microcks-podman.yml
[error] 3-3: trailing spaces
(trailing-spaces)
[error] 92-92: trailing spaces
(trailing-spaces)
[error] 126-126: trailing spaces
(trailing-spaces)
[error] 142-142: trailing spaces
(trailing-spaces)
[error] 151-151: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test generator as dependency with Node 18
- GitHub Check: Test generator as dependency with Node 20
🔇 Additional comments (10)
.gitignore (1)
21-21: LGTM! Properly ignoring Microcks data directory.Adding
microcks-datato the gitignore file is appropriate as it prevents committing test data generated by Microcks during integration testing.packages/templates/clients/websocket/javascript/test/__fixtures__/asyncapi-postman-echo.yml (1)
29-30: Good improvement to the AsyncAPI specification.Adding the messages property to the sendEchoMessage operation explicitly defines which message type is being sent during this operation, improving the clarity of the API contract.
packages/templates/clients/websocket/javascript/components/SendEchoMessage.js (1)
7-14: Well-documented method with clear JSDoc comments.The addition of comprehensive JSDoc comments explains the purpose and parameters of the method, which improves code maintainability.
packages/templates/clients/websocket/test/javascript/package.json (2)
7-17: Good dependency management.
ThedevDependenciessection lists all necessary tools (Babel, ESLint, Jest, etc.) with explicit versions. This is good for consistency in development environments.
18-33: Comprehensive Jest configuration.
The Jest configuration (module file extensions, transform rule, and module name mappings) is well specified to support both ES modules and legacy modules.packages/templates/clients/websocket/javascript/test/__fixtures__/asyncapi-hoppscotch-echo.yml (1)
47-49: Clear addition of example for timestamp message.
The newexamplessection with thesomeRandomDateexample provides a concrete example of the expected payload format. This will help users understand the message schema better.packages/templates/clients/websocket/test/__fixtures__/asyncapi-hoppscotch-server.yml (2)
1-13: Solid AsyncAPI specification layout for server mocks.
The general structure (asyncapi version, info, servers) is clear and provides all necessary information for the mock server.
29-43: Operation definitions are clear and appropriately reference components.
The operations for sending timestamp messages and handling echo messages are correctly linked to the channels and components, providing a well-integrated specification.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 38-38: trailing spaces
(trailing-spaces)
packages/templates/clients/websocket/test/microcks-setup/checkMicrocksReady.sh (2)
1-16: Solid implementation of container health-check logic.
The use ofpodman inspectand the looping mechanism in theall_healthyfunction is clear and serves the intended purpose.
18-26: Clear and user-friendly waiting loop.
The loop that waits for all containers to become healthy, with a 5-second delay, is straightforward and well commented.
| static sendEchoMessage(message, socket) { | ||
| if (socket) this.websocket = socket; | ||
| this.websocket.send(JSON.stringify(message)); |
There was a problem hiding this comment.
Fix potential this context issue in static method.
There's an issue with using this.websocket within a static method. In JavaScript, this in a static method refers to the class itself, not an instance, which means this.websocket might be undefined.
Consider one of these approaches:
-static sendEchoMessage(message, socket) {
- if (socket) this.websocket = socket;
- this.websocket.send(JSON.stringify(message));
+static sendEchoMessage(message, socket) {
+ // Use the provided socket or throw an error if none is provided
+ if (!socket) {
+ throw new Error('WebSocket connection is required');
+ }
+ socket.send(JSON.stringify(message));Or if maintaining backward compatibility with instance usage is needed:
-static sendEchoMessage(message, socket) {
- if (socket) this.websocket = socket;
- this.websocket.send(JSON.stringify(message));
+static sendEchoMessage(message, socket) {
+ const websocket = socket || this.websocket;
+ if (!websocket) {
+ throw new Error('WebSocket connection is required');
+ }
+ websocket.send(JSON.stringify(message));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static sendEchoMessage(message, socket) { | |
| if (socket) this.websocket = socket; | |
| this.websocket.send(JSON.stringify(message)); | |
| static sendEchoMessage(message, socket) { | |
| // Use the provided socket or throw an error if none is provided | |
| if (!socket) { | |
| throw new Error('WebSocket connection is required'); | |
| } | |
| socket.send(JSON.stringify(message)); | |
| } |
| `async def send_message(self, message): | ||
| """ | ||
| Automatically process the outgoing message with registered processors and send it | ||
| using the active WebSocket connection. | ||
| Send a message using the WebSocket connection attached to this instance. | ||
|
|
||
| Args: | ||
| message (dict or str): The message to send. Will be serialized to JSON if it's a dictionary. | ||
|
|
||
| Raises: | ||
| Exception: If sending fails or the socket is not connected. | ||
| """ | ||
| await self._send(message, self.ws_app) | ||
|
|
||
| @staticmethod | ||
| async def send_message_static(message, socket): | ||
| """ | ||
| Send a message using a provided WebSocket connection, without needing an instance. | ||
|
|
||
| Args: | ||
| message (dict or str): The message to send. | ||
| socket (websockets.WebSocketCommonProtocol): The WebSocket to send through. | ||
|
|
||
| Raises: | ||
| Exception: If sending fails or the socket is not connected. | ||
| """ | ||
| await HoppscotchEchoWebSocketClient._send(message, socket) | ||
|
|
||
| @staticmethod | ||
| async def _send(message, socket): | ||
| """ | ||
| Internal helper to handle the actual sending logic. | ||
|
|
||
| Args: | ||
| message (dict or str): The message to send. | ||
| socket (websockets.WebSocketCommonProtocol): The WebSocket to send through. | ||
|
|
||
| Notes: | ||
| If message is a dictionary, it will be automatically converted to JSON. | ||
| """ | ||
| # Apply outgoing processors sequentially. | ||
| for processor in self.outgoing_processors: | ||
| message = processor(message) | ||
|
|
||
| if self.ws_app and self.ws_app.sock and self.ws_app.sock.connected: | ||
| try: | ||
| self.ws_app.send(json.dumps(message)) | ||
| print("\\033[92mSent:\\033[0m", message) | ||
| except Exception as error: | ||
| self.handle_error(error) | ||
| else: | ||
| print("Error: WebSocket is not connected.")` | ||
| try: | ||
| if isinstance(message, dict): | ||
| message = json.dumps(message) | ||
| await socket.send(message) | ||
| except Exception as e: | ||
| print("Error sending:", e)` |
There was a problem hiding this comment.
Fix missing json import and class name inconsistency.
The updated code has two issues that need addressing:
- Missing
jsonimport forjson.dumps()operation in line 47 - Reference to
HoppscotchEchoWebSocketClientin line 31 which might be inconsistent with the actual class name
async def send_message(self, message):
"""
Send a message using the WebSocket connection attached to this instance.
Args:
message (dict or str): The message to send. Will be serialized to JSON if it's a dictionary.
Raises:
Exception: If sending fails or the socket is not connected.
"""
await self._send(message, self.ws_app)
@staticmethod
async def send_message_static(message, socket):
"""
Send a message using a provided WebSocket connection, without needing an instance.
Args:
message (dict or str): The message to send.
socket (websockets.WebSocketCommonProtocol): The WebSocket to send through.
Raises:
Exception: If sending fails or the socket is not connected.
"""
- await HoppscotchEchoWebSocketClient._send(message, socket)
+ await SendEchoMessage._send(message, socket)
@staticmethod
async def _send(message, socket):
"""
Internal helper to handle the actual sending logic.
Args:
message (dict or str): The message to send.
socket (websockets.WebSocketCommonProtocol): The WebSocket to send through.
Notes:
If message is a dictionary, it will be automatically converted to JSON.
"""
+ import json
try:
if isinstance(message, dict):
message = json.dumps(message)
await socket.send(message)
except Exception as e:
print("Error sending:", e)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| `async def send_message(self, message): | |
| """ | |
| Automatically process the outgoing message with registered processors and send it | |
| using the active WebSocket connection. | |
| Send a message using the WebSocket connection attached to this instance. | |
| Args: | |
| message (dict or str): The message to send. Will be serialized to JSON if it's a dictionary. | |
| Raises: | |
| Exception: If sending fails or the socket is not connected. | |
| """ | |
| await self._send(message, self.ws_app) | |
| @staticmethod | |
| async def send_message_static(message, socket): | |
| """ | |
| Send a message using a provided WebSocket connection, without needing an instance. | |
| Args: | |
| message (dict or str): The message to send. | |
| socket (websockets.WebSocketCommonProtocol): The WebSocket to send through. | |
| Raises: | |
| Exception: If sending fails or the socket is not connected. | |
| """ | |
| await HoppscotchEchoWebSocketClient._send(message, socket) | |
| @staticmethod | |
| async def _send(message, socket): | |
| """ | |
| Internal helper to handle the actual sending logic. | |
| Args: | |
| message (dict or str): The message to send. | |
| socket (websockets.WebSocketCommonProtocol): The WebSocket to send through. | |
| Notes: | |
| If message is a dictionary, it will be automatically converted to JSON. | |
| """ | |
| # Apply outgoing processors sequentially. | |
| for processor in self.outgoing_processors: | |
| message = processor(message) | |
| if self.ws_app and self.ws_app.sock and self.ws_app.sock.connected: | |
| try: | |
| self.ws_app.send(json.dumps(message)) | |
| print("\\033[92mSent:\\033[0m", message) | |
| except Exception as error: | |
| self.handle_error(error) | |
| else: | |
| print("Error: WebSocket is not connected.")` | |
| try: | |
| if isinstance(message, dict): | |
| message = json.dumps(message) | |
| await socket.send(message) | |
| except Exception as e: | |
| print("Error sending:", e)` | |
| async def send_message(self, message): | |
| """ | |
| Send a message using the WebSocket connection attached to this instance. | |
| Args: | |
| message (dict or str): The message to send. Will be serialized to JSON if it's a dictionary. | |
| Raises: | |
| Exception: If sending fails or the socket is not connected. | |
| """ | |
| await self._send(message, self.ws_app) | |
| @staticmethod | |
| async def send_message_static(message, socket): | |
| """ | |
| Send a message using a provided WebSocket connection, without needing an instance. | |
| Args: | |
| message (dict or str): The message to send. | |
| socket (websockets.WebSocketCommonProtocol): The WebSocket to send through. | |
| Raises: | |
| Exception: If sending fails or the socket is not connected. | |
| """ | |
| await SendEchoMessage._send(message, socket) | |
| @staticmethod | |
| async def _send(message, socket): | |
| """ | |
| Internal helper to handle the actual sending logic. | |
| Args: | |
| message (dict or str): The message to send. | |
| socket (websockets.WebSocketCommonProtocol): The WebSocket to send through. | |
| Notes: | |
| If message is a dictionary, it will be automatically converted to JSON. | |
| """ | |
| import json | |
| try: | |
| if isinstance(message, dict): | |
| message = json.dumps(message) | |
| await socket.send(message) | |
| except Exception as e: | |
| print("Error sending:", e) |
| microcks-cli import __fixtures__/asyncapi-hoppscotch-server.yml \ | ||
| --microcksURL=http://localhost:8080/api/ \ | ||
| --keycloakClientId=microcks-serviceaccount \ | ||
| --keycloakClientSecret="ab54d329-e435-41ae-a900-ec6b3fe15c54" |
There was a problem hiding this comment.
Security concern: Hardcoded secrets in documentation.
The README contains hardcoded API keys/secrets which should never be included in documentation or repositories.
Replace the hardcoded client secret with a placeholder instruction to get the actual secret from a secure source or environment variable:
- --keycloakClientSecret="ab54d329-e435-41ae-a900-ec6b3fe15c54"
+ --keycloakClientSecret="<your-client-secret>"And similarly for the second occurrence:
- --keycloakClientSecret="ab54d329-e435-41ae-a900-ec6b3fe15c54" \
+ --keycloakClientSecret="<your-client-secret>" \Consider adding a note about securely obtaining this secret.
Also applies to: 68-68
🧰 Tools
🪛 Gitleaks (8.21.2)
50-50: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
| server = await websockets.serve(handler, port=port) | ||
| await asyncio.sleep(1) # Give server time to start |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure server starts properly before proceeding
You're waiting for the server to start, but not checking if it started successfully. Consider adding verification logic to ensure the server is ready before proceeding with the test.
# Start the WebSocket server
server = await websockets.serve(handler, port=port)
await asyncio.sleep(1) # Give server time to start
+# Verify server is running
+assert server.is_serving(), "WebSocket server failed to start"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| server = await websockets.serve(handler, port=port) | |
| await asyncio.sleep(1) # Give server time to start | |
| server = await websockets.serve(handler, port=port) | |
| await asyncio.sleep(1) # Give server time to start | |
| assert server.is_serving(), "WebSocket server failed to start" |
| # Poll Microcks for result | ||
| success = False | ||
| for _ in range(30): | ||
| result = requests.get(f"{microcks_test_endpoint}/{test_id}").json() | ||
| print("Polling Microcks:", result) | ||
| if result.get("success") is True: | ||
| success = True | ||
| break | ||
| await asyncio.sleep(2) | ||
| #await asyncio.sleep(1000) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve polling mechanism with better error handling
The polling loop lacks explanatory comments about the 30 iterations limit, and there's no error handling if the API response fails.
# Poll Microcks for result
success = False
+max_attempts = 30 # Set timeout: 30 attempts * 2 seconds = 60 seconds total
-for _ in range(30):
+for attempt in range(max_attempts):
+ try:
result = requests.get(f"{microcks_test_endpoint}/{test_id}").json()
print("Polling Microcks:", result)
if result.get("success") is True:
success = True
break
+ except Exception as e:
+ print(f"Error polling Microcks (attempt {attempt+1}/{max_attempts}): {e}")
await asyncio.sleep(2)
-#await asyncio.sleep(1000)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Poll Microcks for result | |
| success = False | |
| for _ in range(30): | |
| result = requests.get(f"{microcks_test_endpoint}/{test_id}").json() | |
| print("Polling Microcks:", result) | |
| if result.get("success") is True: | |
| success = True | |
| break | |
| await asyncio.sleep(2) | |
| #await asyncio.sleep(1000) | |
| # Poll Microcks for result | |
| success = False | |
| max_attempts = 30 # Set timeout: 30 attempts * 2 seconds = 60 seconds total | |
| for attempt in range(max_attempts): | |
| try: | |
| result = requests.get(f"{microcks_test_endpoint}/{test_id}").json() | |
| print("Polling Microcks:", result) | |
| if result.get("success") is True: | |
| success = True | |
| break | |
| except Exception as e: | |
| print(f"Error polling Microcks (attempt {attempt+1}/{max_attempts}): {e}") | |
| await asyncio.sleep(2) |
| #await asyncio.sleep(1000) | ||
| server.close() | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use a finally block to ensure server cleanup
The server should be closed even if assertions fail. Use a try/finally pattern to ensure proper cleanup.
-#await asyncio.sleep(1000)
-server.close()
+# Clean up server resources regardless of test outcome
+try:
+ assert success, f"Microcks test {test_id} did not succeed"
+finally:
+ server.close()
+ await server.wait_closed() # Ensure proper shutdown
-assert success, f"Microcks test {test_id} did not succeed"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #await asyncio.sleep(1000) | |
| server.close() | |
| # Clean up server resources regardless of test outcome | |
| try: | |
| assert success, f"Microcks test {test_id} did not succeed" | |
| finally: | |
| server.close() | |
| await server.wait_closed() # Ensure proper shutdown |
| sh -c "microcks-cli import __fixtures__/asyncapi-hoppscotch-server.yml | ||
| --microcksURL=http://microcks:8080/api/ | ||
| --keycloakClientId=microcks-serviceaccount | ||
| --keycloakClientSecret=ab54d329-e435-41ae-a900-ec6b3fe15c54" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Security concern: Hardcoded credentials
The configuration contains a hardcoded client secret that appears to be a valid credential.
Consider using environment variables or a secrets management solution instead of hardcoding credentials in the configuration file:
- --keycloakClientSecret=ab54d329-e435-41ae-a900-ec6b3fe15c54"
+ --keycloakClientSecret=${KEYCLOAK_CLIENT_SECRET:-dummy-secret}"🏁 Script executed:
#!/bin/bash
# Check if this appears to be a real UUID
CLIENT_SECRET="ab54d329-e435-41ae-a900-ec6b3fe15c54"
# Validate if this is a UUID format (which suggests it might be a real credential)
if [[ $CLIENT_SECRET =~ ^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$ ]]; then
echo "WARNING: The string appears to be a valid UUID format, which suggests it could be a real credential"
# Search for other instances of this secret in the codebase
echo "Checking for other instances of this secret in the codebase:"
rg -i "$CLIENT_SECRET" --type yaml --type js --type ts --type json --type py
else
echo "The string doesn't match UUID format, might be a placeholder"
fi
# Check if it's mentioned in documentation or README files
echo "Checking if the token is documented:"
rg -i "ab54d329|keycloakClientSecret" --type md --type txtLength of output: 1130
Critical Security Vulnerability: Hardcoded Client Secret Detected
The analysis confirms that the client secret in question is a valid UUID and is hardcoded in multiple locations, both in the Podman YAML configuration and the README documentation. Hardcoding such credentials risks exposing sensitive information.
Affected files:
packages/templates/clients/websocket/test/microcks-setup/microcks-podman.yml(line 108)packages/templates/clients/websocket/test/README.md
Recommendation:
Replace the hardcoded secret with an environment variable or integrate a secrets management solution. For example:
- --keycloakClientSecret=ab54d329-e435-41ae-a900-ec6b3fe15c54"
+ --keycloakClientSecret=${KEYCLOAK_CLIENT_SECRET:-dummy-secret}"Please ensure that similar changes are applied to any other instances where this secret is used or documented.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| --keycloakClientSecret=ab54d329-e435-41ae-a900-ec6b3fe15c54" | |
| --keycloakClientSecret=${KEYCLOAK_CLIENT_SECRET:-dummy-secret}" |
🧰 Tools
🪛 Gitleaks (8.21.2)
108-108: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
| async function waitForTestSuccess(url) { | ||
| while (true) { | ||
| try { | ||
| const response = await fetch(url, { | ||
| method: 'GET', | ||
| headers: { 'Content-Type': 'application/json' } | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| console.error(`Request failed with status: ${response.status}`); | ||
| break; | ||
| } | ||
|
|
||
| const data = await response.json(); | ||
|
|
||
| if (data.success) { | ||
| return true; | ||
| } | ||
| } catch (err) { | ||
| break; | ||
| } | ||
|
|
||
| await new Promise(resolve => setTimeout(resolve, 2000)); // Wait 2 seconds | ||
| } | ||
|
|
||
| return false; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add timeout parameter to prevent infinite polling
The waitForTestSuccess function lacks a timeout parameter, which could lead to an infinite loop if the server never responds with success.
/**
* Polls the Microcks API every 2 seconds until a test with the given ID reports success.
*
* @param {string} url - link to endpoint providing info on particular test
+ * @param {number} timeout - Maximum time (in ms) to wait before giving up. Default is 60000ms (1 minute).
* @returns {Promise<boolean>} Resolves with `true` when the test is marked as successful.
*/
-async function waitForTestSuccess(url) {
+async function waitForTestSuccess(url, timeout = 60000) {
+ const startTime = Date.now();
while (true) {
+ // Check if we've exceeded the timeout
+ if (Date.now() - startTime > timeout) {
+ console.error(`Test polling timed out after ${timeout}ms`);
+ return false;
+ }
+
try {
const response = await fetch(url, {
method: 'GET',
headers: { 'Content-Type': 'application/json' }
});
if (!response.ok) {
console.error(`Request failed with status: ${response.status}`);
- break;
+ // Continue polling despite error
+ await new Promise(resolve => setTimeout(resolve, 2000));
+ continue;
}
const data = await response.json();
if (data.success) {
return true;
}
} catch (err) {
- break;
+ console.error(`Error polling test status: ${err.message}`);
+ // Continue polling despite error
+ await new Promise(resolve => setTimeout(resolve, 2000));
+ continue;
}
await new Promise(resolve => setTimeout(resolve, 2000)); // Wait 2 seconds
}
-
- return false;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async function waitForTestSuccess(url) { | |
| while (true) { | |
| try { | |
| const response = await fetch(url, { | |
| method: 'GET', | |
| headers: { 'Content-Type': 'application/json' } | |
| }); | |
| if (!response.ok) { | |
| console.error(`Request failed with status: ${response.status}`); | |
| break; | |
| } | |
| const data = await response.json(); | |
| if (data.success) { | |
| return true; | |
| } | |
| } catch (err) { | |
| break; | |
| } | |
| await new Promise(resolve => setTimeout(resolve, 2000)); // Wait 2 seconds | |
| } | |
| return false; | |
| } | |
| /** | |
| * Polls the Microcks API every 2 seconds until a test with the given ID reports success. | |
| * | |
| * @param {string} url - link to endpoint providing info on particular test | |
| * @param {number} timeout - Maximum time (in ms) to wait before giving up. Default is 60000ms (1 minute). | |
| * @returns {Promise<boolean>} Resolves with `true` when the test is marked as successful. | |
| */ | |
| async function waitForTestSuccess(url, timeout = 60000) { | |
| const startTime = Date.now(); | |
| while (true) { | |
| // Check if we've exceeded the timeout | |
| if (Date.now() - startTime > timeout) { | |
| console.error(`Test polling timed out after ${timeout}ms`); | |
| return false; | |
| } | |
| try { | |
| const response = await fetch(url, { | |
| method: 'GET', | |
| headers: { 'Content-Type': 'application/json' } | |
| }); | |
| if (!response.ok) { | |
| console.error(`Request failed with status: ${response.status}`); | |
| // Continue polling despite error | |
| await new Promise(resolve => setTimeout(resolve, 2000)); | |
| continue; | |
| } | |
| const data = await response.json(); | |
| if (data.success) { | |
| return true; | |
| } | |
| } catch (err) { | |
| console.error(`Error polling test status: ${err.message}`); | |
| // Continue polling despite error | |
| await new Promise(resolve => setTimeout(resolve, 2000)); | |
| continue; | |
| } | |
| await new Promise(resolve => setTimeout(resolve, 2000)); // Wait 2 seconds | |
| } | |
| } |
|
you Rabbit, are you sure you don't have anything more interesting to do, rather than commenting my PR 😄 |
|
@derberg I tried running setup locally. # the higher timeout the more test samples will run
microcks-cli test 'Hoppscotch WebSocket Server:1.0.0' localhost:8081/api/ws/Hoppscotch+WebSocket+Server/1.0.0/sendTimeStampMessage ASYNC_API_SCHEMA \
--microcksURL=http://localhost:8080/api/ \
--insecure \
--waitFor=15sec \
--keycloakClientId=microcks-serviceaccount \
--keycloakClientSecret="ab54d329-e435-41ae-a900-ec6b3fe15c54" \
--filteredOperations="[\"SEND sendTimeStampMessage\"]"When I added
|
fixes: #1417
we use microcks and podman for efficient testing that can even run on local
we test send and receive messages - both python and javascript
it is not automated and optimized - as it was not the goal of that task - the goal was to find the best solution and this is done and works. Rest marked with
TODOin codeSummary by CodeRabbit
New Features
Documentation
Tests
Chore