packages/templates/clients/websocket/javascript/test/integration.test.js (7)

32-33: 🛠️ Refactor suggestion

Update test to use the new directory structure.

The test still reads the client file from the old location. Consider updating it to use the new testResultPathPostman constant.

-    const client = await readFile(path.join(testResultPath, testOutputFile), 'utf8');
+    const client = await readFile(path.join(testResultPathPostman, testOutputFile), 'utf8');
     expect(client).toMatchSnapshot();

---

49-50: 🛠️ Refactor suggestion

Update test to use the new directory structure.

Similar to the previous test, this one also reads the client file from the old location. Consider updating it to use the new testResultPathHoppscotch constant.

-    const client = await readFile(path.join(testResultPath, testOutputFile), 'utf8');
+    const client = await readFile(path.join(testResultPathHoppscotch, testOutputFile), 'utf8');
     expect(client).toMatchSnapshot();

---

65-69: 🛠️ Refactor suggestion

Update test to use the new directory structure.

This test should also be updated to use the new directory structure for consistency.

-    const clientOutputFile = path.join(testResultPath, defaultOutputFile);
+    const clientOutputFile = path.join(testResultPathClient, defaultOutputFile);
 
     const checkClientOutputFileExists = await stat(clientOutputFile);
 
     expect(checkClientOutputFileExists.isFile()).toBeTruthy();

---

22-28: 🛠️ Refactor suggestion

Update Generator initialization to use the new path constants.

For the first test case, update the Generator initialization to use the new testResultPathPostman constant.

-    const generator = new Generator(template, testResultPath, {
+    const generator = new Generator(template, testResultPathPostman, {
       forceWrite: true,
       templateParams: {
         server: 'echoServer',
         clientFileName: testOutputFile
       }
     });

---

39-45: 🛠️ Refactor suggestion

Update Generator initialization to use the new path constants.

For the second test case, update the Generator initialization to use the new testResultPathHoppscotch constant.

-    const generator = new Generator(template, testResultPath, {
+    const generator = new Generator(template, testResultPathHoppscotch, {
       forceWrite: true,
       templateParams: {
         server: 'echoServer',
         clientFileName: testOutputFile
       }
     });

---

56-61: 🛠️ Refactor suggestion

Update Generator initialization to use the new path constants.

For the third test case, update the Generator initialization to use the new testResultPathClient constant.

-    const generator = new Generator(template, testResultPath, {
+    const generator = new Generator(template, testResultPathClient, {
       forceWrite: true,
       templateParams: {
         server: 'echoServer',
       }
     });

---

75-80: 🛠️ Refactor suggestion

Update Generator initialization to use the new path constants.

For consistency, update the Generator initialization in the fourth test case as well.

-    const generator = new Generator(template, testResultPath, {
+    const generator = new Generator(template, testResultPathHoppscotch, {
       forceWrite: true,
       templateParams: {
         clientFileName: testOutputFile
       }
     });

packages/templates/clients/websocket/javascript/README.md (1)

9-9: Improve readability with proper punctuation and articles.

The added explanatory text would benefit from improved grammar to enhance clarity for users.

-> By default this is testing against Postman echo service. You can modify `packages/templates/clients/websocket/javascript/example.js` and change first line to `const WSClient = require('./test/temp/snapshotTestResult/hoppscotchClient/client-hoppscotch.js');` and run `node example.js` again. You will see example still works but now it is using a different API. This is possible since both AsyncAPI documents define the same name of operation for sending messages: `sendEchoMessage` so each client generated has the same API.
+> By default, this is testing against Postman echo service. You can modify `packages/templates/clients/websocket/javascript/example.js` and change the first line to `const WSClient = require('./test/temp/snapshotTestResult/hoppscotchClient/client-hoppscotch.js');` and run `node example.js` again. You will see the example still works, but now it is using a different API. This is possible since both AsyncAPI documents define the same name of operation for sending messages: `sendEchoMessage` so each client generated has the same API.

🧰 Tools

🪛 LanguageTool

[uncategorized] ~9-~9: Did you mean: “By default,”?
Context: ...rated by the test: node example.js > By default this is testing against Postman echo se...

(BY_DEFAULT_COMMA)

---

[uncategorized] ~9-~9: You might be missing the article “the” here.
Context: ...ocket/javascript/example.jsand change first line toconst WSClient = require('./te...

(AI_EN_LECTOR_MISSING_DETERMINER_THE)

---

[uncategorized] ~9-~9: Possible missing article found.
Context: ...n node example.js again. You will see example still works but now it is using a diffe...

(AI_HYDRA_LEO_MISSING_THE)

---

[uncategorized] ~9-~9: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... again. You will see example still works but now it is using a different API. This i...

(COMMA_COMPOUND_SENTENCE)

packages/templates/clients/websocket/javascript/test/temp/snapshotTestResult/postmanClient/client-postman.js (4)

12-12: Consider simplifying the class name to avoid redundancy.

The class name PostmanEchoWebSocketClientClient contains a redundant "Client" suffix.

-class PostmanEchoWebSocketClientClient {
+class PostmanEchoWebSocketClient {

Remember to also update the export statement at the end of the file:

-module.exports = PostmanEchoWebSocketClientClient;
+module.exports = PostmanEchoWebSocketClient;

---

61-68: Fix method documentation comment.

The comment above this method incorrectly states it's for registering error handlers, but it's actually for message handlers.

-  // Method to register custom error handlers
+  // Method to register custom message handlers
   registerMessageHandler(handler) {
     if (typeof handler === 'function') {
       this.messageHandlers.push(handler);
     } else {
       console.warn('Message handler must be a function');
     }
   }

---

25-29: Add connection state tracking.

The connect method doesn't track the connection state, which could lead to multiple connection attempts or attempts to use a disconnected socket.

   connect() {
     return new Promise((resolve, reject) => {
+      if (this.websocket && this.websocket.readyState === WebSocket.OPEN) {
+        console.log('Already connected to Postman Echo WebSocket Client server');
+        resolve();
+        return;
+      }
+
       this.websocket = new WebSocket(this.url);

       // On successful connection
       this.websocket.onopen = () => {
         console.log('Connected to Postman Echo WebSocket Client server');
         resolve();
       };

---

90-96: Return a Promise from the close method for consistency.

For consistency with the connect method, consider making the close method return a Promise as well.

   // Method to close the WebSocket connection
   close() {
-    if (this.websocket) {
-      this.websocket.close();
-      console.log('WebSocket connection closed.');
-    }
+    return new Promise((resolve) => {
+      if (!this.websocket) {
+        console.log('No WebSocket connection to close.');
+        resolve();
+        return;
+      }
+      
+      this.websocket.onclose = () => {
+        console.log('WebSocket connection closed.');
+        this.websocket = null;
+        resolve();
+      };
+      
+      this.websocket.close();
+    });
   }

packages/templates/clients/websocket/javascript/test/temp/snapshotTestResult/postmanClient/README.md (1)

37-44: Consider adding response type information.

The documentation for sendEchoMessage could be enhanced by specifying what type of response to expect after sending a message. This would help users understand the complete request-response cycle.

packages/templates/clients/websocket/javascript/test/temp/snapshotTestResult/hoppscotchClient/README.md (2)

5-5: Fix typo in overview section.

There's a typo in "Undestand" which should be "Understand".

-Undestand how to use Hoppscotch Echo WebSocket as a client. Hoppscotch Echo WebSocket server echoes back any messages sent to it. You can use this to test WebSocket connections and message flows.
+Understand how to use Hoppscotch Echo WebSocket as a client. Hoppscotch Echo WebSocket server echoes back any messages sent to it. You can use this to test WebSocket connections and message flows.

---

39-39: Add missing article in the sentence.

Missing "the" before "server" makes the sentence grammatically incorrect.

-Receive the timestamp message sent from server every second.
+Receive the timestamp message sent from the server every second.

🧰 Tools

🪛 LanguageTool

[uncategorized] ~39-~39: You might be missing the article “the” here.
Context: ...Receive the timestamp message sent from server every second. Example: ```javascr...

(AI_EN_LECTOR_MISSING_DETERMINER_THE)

packages/templates/clients/websocket/javascript/test/temp/snapshotTestResult/hoppscotchClient/client-hoppscotch.js (3)

59-60: Correct method comment for registerMessageHandler.

The method comment incorrectly states this is for registering error handlers, but it's actually for message handlers.

-  // Method to register custom error handlers
+  // Method to register custom message handlers
  registerMessageHandler(handler) {

---

83-85: Consider JSON handling consistency with documentation.

The sendEchoMessage method uses JSON.stringify on all messages, but the README example (line 53) implies sending a simple string. This inconsistency could cause confusion. Consider either:

1. Updating the method to handle both string and object types, or
2. Clarifying in the documentation that all messages are automatically JSON-stringified

  sendEchoMessage(message) {
-    this.websocket.send(JSON.stringify(message));
+    const payload = typeof message === 'string' ? message : JSON.stringify(message);
+    this.websocket.send(payload);
    console.log('Sent message to echo server:', message);
  }

---

77-80: Simplify the handleMessage method.

The current implementation is a simple wrapper that only calls the callback if it exists. This pattern is already used in other methods like the WebSocket's onmessage handler. Consider inlining this logic to reduce unnecessary abstraction.

-  // Method to handle message with callback
-  handleMessage(message, cb) {
-    if (cb) cb(message);
-  }

And update line 35:

-            this.handleMessage(event.data, handler);
+            handler(event.data);

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

packages/templates/clients/websocket/javascript/README.md (1)

9-9: Refine README text for clarity and minor grammatical improvements.

Consider updating this line by inserting a comma after "By default", adding the definite article "the" before "Postman echo service" and before "first line", and inserting a comma before "but" to separate independent clauses. This will improve readability and conform to standard English grammar. For example:

- > By default this is testing against Postman echo service. You can modify `packages/templates/clients/websocket/javascript/example.js` and change first line to `const WSClient = require('./test/temp/snapshotTestResult/hoppscotch-client/client-hoppscotch.js');` again. You will see example still works but now it is using a different API. This is possible since both AsyncAPI documents define the same name of operation for sending messages: `sendEchoMessage` so each client generated has the same API.
+ > By default, this is testing against the Postman echo service. You can modify `packages/templates/clients/websocket/javascript/example.js` and change the first line to `const WSClient = require('./test/temp/snapshotTestResult/hoppscotch-client/client-hoppscotch.js');` again. You will see that the example still works, but now it is using a different API. This is possible since both AsyncAPI documents define the same name of operation for sending messages: `sendEchoMessage`, so each client generated has the same API.

🧰 Tools

🪛 LanguageTool

[uncategorized] ~9-~9: Did you mean: “By default,”?
Context: ...rated by the test: node example.js > By default this is testing against Postman echo se...

(BY_DEFAULT_COMMA)

---

[uncategorized] ~9-~9: You might be missing the article “the” here.
Context: ...ocket/javascript/example.jsand change first line toconst WSClient = require('./te...

(AI_EN_LECTOR_MISSING_DETERMINER_THE)

---

[uncategorized] ~9-~9: Possible missing article found.
Context: ...t-hoppscotch.js');` again. You will see example still works but now it is using a diffe...

(AI_HYDRA_LEO_MISSING_THE)

---

[uncategorized] ~9-~9: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... again. You will see example still works but now it is using a different API. This i...

(COMMA_COMPOUND_SENTENCE)

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

packages/templates/clients/websocket/javascript/README.md (1)

9-9: Improve Documentation Clarity & Grammar in the Example Note

The updated example now correctly points to the postman-client library, resolving the earlier naming discrepancy. However, there are a few grammatical improvements to boost clarity and readability:

• Insert a comma after “By default” and add the article “the” before “Postman Echo service.”
• Consider removing the word “again” after the import path instruction.
• Insert a comma before “but” in the sentence “You will see example still works but now it is using a different API.” This will better separate the independent clauses.

For instance, below is a suggested diff to refine the text:

- > By default this is testing against Postman echo service. You can modify `packages/templates/clients/websocket/javascript/example.js` and change first line to `const WSClient = require('./test/temp/snapshotTestResult/postman-client/client-postman.js');` again. You will see example still works but now it is using a different API. This is possible since both AsyncAPI documents define the same name of operation for sending messages: `sendEchoMessage` so each client generated has the same API.
+ > By default, this is testing against the Postman Echo service. You can modify `packages/templates/clients/websocket/javascript/example.js` and change the first line to `const WSClient = require('./test/temp/snapshotTestResult/postman-client/client-postman.js');`. You will see that the example still works, but now it uses a different API. This is possible since both AsyncAPI documents define the same operation name (`sendEchoMessage`), so each generated client has the same API.

These changes will ensure the README is both technically correct and easier to understand.

🧰 Tools

🪛 LanguageTool

[uncategorized] ~9-~9: Did you mean: “By default,”?
Context: ...rated by the test: node example.js > By default this is testing against Postman echo se...

(BY_DEFAULT_COMMA)

---

[uncategorized] ~9-~9: You might be missing the article “the” here.
Context: ...ocket/javascript/example.jsand change first line toconst WSClient = require('./te...

(AI_EN_LECTOR_MISSING_DETERMINER_THE)

---

[uncategorized] ~9-~9: Possible missing article found.
Context: ...ient-postman.js');` again. You will see example still works but now it is using a diffe...

(AI_HYDRA_LEO_MISSING_THE)

---

[uncategorized] ~9-~9: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... again. You will see example still works but now it is using a different API. This i...

(COMMA_COMPOUND_SENTENCE)

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

packages/templates/clients/websocket/javascript/README.md (1)

9-9: Enhance sentence clarity and formatting

• Insert a comma after “By default,” and before “but” for proper compound-sentence punctuation.
• Wrap the require statement in a fenced code block to improve readability.

Suggested diff:

> By default, this is testing against the Postman echo service. You can modify `packages/templates/clients/websocket/javascript/example.js` and change the first line to:

+```js
+const WSClient = require('./test/temp/snapshotTestResult/client-postman/client.js');
+```

>You will see the example still works, but now it is using a different API. This is possible since both AsyncAPI documents define the same operation name for sending messages: `sendEchoMessage`, so each client generated has the same API.

🧰 Tools

🪛 LanguageTool

[uncategorized] ~9-~9: Did you mean: “By default,”?
Context: ...rated by the test: node example.js > By default this is testing against Postman echo se...

(BY_DEFAULT_COMMA)

---

[uncategorized] ~9-~9: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...in. You will see the example still works but now it is using a different API. This i...

(COMMA_COMPOUND_SENTENCE)

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

packages/templates/clients/websocket/javascript/README.md (1)

9-9: Improve clarity and punctuation in the instruction.

Add a comma after “By default”, a comma before “but” in the second sentence, and consider adding “the” before “Hoppscotch echo service” for consistency. For example:

- > By default this is testing against Hoppscotch echo service. You can modify `packages/templates/clients/websocket/javascript/example.js` and change the first line to `const WSClient = require('./test/temp/snapshotTestResult/custom-client-hoppscotch.js');` and run `node example.js` again. You will see the example still works but now it is using a different API. This is possible since both AsyncAPI documents define the same name of operation for sending messages: `sendEchoMessage`, so each client generated has the same API.
+ > By default, this is testing against the Hoppscotch echo service. You can modify `packages/templates/clients/websocket/javascript/example.js` and change the first line to:
+ > 
+ > ```js
+ > const WSClient = require('./test/temp/snapshotTestResult/custom-client-hoppscotch.js');
+ > ```
+ > 
+ > Then run `node example.js` again. You will see the example still works, but now it is using a different API. This is possible since both AsyncAPI documents define the same operation name for sending messages (`sendEchoMessage`), so each generated client has the same API.

<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 LanguageTool</summary>

[uncategorized] ~9-~9: Did you mean: “By default,”?
Context: ...rated by the test: `node example.js`  > By default this is testing against Hoppscotch echo...

(BY_DEFAULT_COMMA)

---

[uncategorized] ~9-~9: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...in. You will see the example still works but now it is using a different API. This i...

(COMMA_COMPOUND_SENTENCE)

</details>

</details>

</blockquote></details>

</blockquote></details>

<details>
<summary>📜 Review details</summary>

**Configuration used: CodeRabbit UI**
**Review profile: CHILL**
**Plan: Pro**

<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between f87f1c2b91f014f96639bc787a26caece848e15d and d666c5c92dd37fd0c6677de1cc1ec7d501a01af1.

</details>

<details>
<summary>📒 Files selected for processing (3)</summary>

* `packages/templates/clients/websocket/javascript/README.md` (1 hunks)
* `packages/templates/clients/websocket/javascript/example.js` (2 hunks)
* `packages/templates/clients/websocket/javascript/test/integration.test.js` (5 hunks)

</details>

<details>
<summary>✅ Files skipped from review due to trivial changes (1)</summary>

* packages/templates/clients/websocket/javascript/example.js

</details>

<details>
<summary>🚧 Files skipped from review as they are similar to previous changes (1)</summary>

* packages/templates/clients/websocket/javascript/test/integration.test.js

</details>

<details>
<summary>🧰 Additional context used</summary>

<details>
<summary>🪛 LanguageTool</summary>

<details>
<summary>packages/templates/clients/websocket/javascript/README.md</summary>

[uncategorized] ~9-~9: Did you mean: “By default,”?
Context: ...rated by the test: `node example.js`  > By default this is testing against Hoppscotch echo...

(BY_DEFAULT_COMMA)

---

[uncategorized] ~9-~9: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...in. You will see the example still works but now it is using a different API. This i...

(COMMA_COMPOUND_SENTENCE)

</details>

</details>

</details>

<details>
<summary>⏰ Context from checks skipped due to timeout of 90000ms (2)</summary>

* GitHub Check: Test generator as dependency with Node 20
* GitHub Check: Test generator as dependency with Node 18

</details>

<details>
<summary>🔇 Additional comments (1)</summary><blockquote>

<details>
<summary>packages/templates/clients/websocket/javascript/README.md (1)</summary>

`9-9`: <details>
<summary>❓ Verification inconclusive</summary>

**Verify the example.js require path matches the actual generated output.**

Ensure that `custom-client-hoppscotch.js` exists under `test/temp/snapshotTestResult`. If the directory or filename differs (e.g., `client-hoppscotch/client.js`), update the path accordingly.

---

🏁 Script executed:

```shell
#!/bin/bash
# Verify the presence and path of the generated Hoppscotch client file
fd custom-client-hoppscotch.js packages/templates/clients/websocket/javascript

Length of output: 80

---

🏁 Script executed:

#!/bin/bash
# List any snapshotTestResult directories and their JavaScript contents
find packages/templates/clients/websocket/javascript -type d -name 'snapshotTestResult' -exec echo "Directory: {}" \; -exec find {} -maxdepth 1 -type f -name '*.js' -print \;

# Search the entire repo for the expected generated file
fd custom-client-hoppscotch.js .

# As a fallback, list all files under test/temp to see actual names
find packages/templates/clients/websocket/javascript/test/temp -type f -print

Length of output: 386

---

Confirm README require path for generated client
I couldn’t locate custom-client-hoppscotch.js under packages/templates/clients/websocket/javascript/test/temp/snapshotTestResult. Please verify the actual generated file name and its relative path, then update the example in the README accordingly.

Locations to review:

• packages/templates/clients/websocket/javascript/README.md (line 9)

Suggested diff if the path differs:

- const WSClient = require('./test/temp/snapshotTestResult/custom-client-hoppscotch.js');
+ const WSClient = require('<correct/relative/path/to/generated-client.js>');

🧰 Tools

🪛 LanguageTool

[uncategorized] ~9-~9: Did you mean: “By default,”?
Context: ...rated by the test: node example.js > By default this is testing against Hoppscotch echo...

(BY_DEFAULT_COMMA)

---

[uncategorized] ~9-~9: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...in. You will see the example still works but now it is using a different API. This i...

(COMMA_COMPOUND_SENTENCE)