feat: write tests and refactor scripts #3012

vishvamsinh28 · 2024-05-31T19:04:49Z

This PR sets up Jest for testing Node.js scripts, refactors few existing scripts for better readability and maintainability, and adds initial tests to ensure functionality. It also includes coverage reporting setup and updates .gitignore to exclude coverage files.

netlify · 2024-05-31T19:05:03Z

✅ Deploy Preview for asyncapi-website ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`02faf33`
🔍 Latest deploy log	https://app.netlify.com/sites/asyncapi-website/deploys/6682b4d757e6b10008980fb5
😎 Deploy Preview	https://deploy-preview-3012--asyncapi-website.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

asyncapi-bot · 2024-05-31T19:07:49Z

⚡️ Lighthouse report for the changes in this PR:

Category	Score
🔴 Performance	46
🟢 Accessibility	98
🟢 Best practices	92
🟢 SEO	100
🟠 PWA	56

Lighthouse ran on https://deploy-preview-3012--asyncapi-website.netlify.app/

anshgoyalevil · 2024-06-01T17:49:24Z

scripts/write-json.js

+
+        await writeFile(writePAth,JSON.stringify(jsonContent));
+    }catch(error){
+        console.log(error)


Use console.error for error logging. Update the related tests.

anshgoyalevil · 2024-06-01T17:57:37Z

tests/utils.test.js

+
+describe('convertToJson', () => {
+  test('should return JSON object if input is valid JSON string', () => {
+    const jsonString = '{"name": "AsyncAPI", "age": 5}';


Can you create an actual json file (for tests only), and read its content on the go rather than hard-coding JSON strings like this?

If fs module isn't working in jest, you can even create js objects and use json.stringify() to convert objects to JSON String.

you can create fixtures to store the static data that are needed in tests.

anshgoyalevil · 2024-06-01T18:02:45Z

tests/utils.test.js

+  });
+
+  test('should return JavaScript object if input is valid YAML string', () => {
+    const yamlString = 'name: AsyncAPI\nage: 5';


Try to read actual yaml file here, and iff its not viable, use multiline string. Example:

const yamlString = ` name: AsyncAPI age: 5 `

anshgoyalevil · 2024-06-01T18:04:56Z

tests/utils.test.js

+
+  test('should return null if input is invalid JSON and invalid YAML', () => {
+    const invalidString = 'name: AsyncAPI, age: five';
+    const result = convertToJson(invalidString);


Enclose it in a try catch statement, and add an expect statement for verifying the error.

anshgoyalevil · 2024-06-01T18:12:33Z

tests/write-json.test.js

+  test('should read a file, convert it to JSON, and write the JSON to another file', async () => {
+    const readPath = 'config/testInput.yaml';
+    const writePath = 'config/testOutput.json';
+    const fileContent = 'name: AsyncAPI\nage: 5';


Use multiline strings for better readability.

anshgoyalevil · 2024-06-01T18:15:55Z

tests/write-json.test.js

+  test('should log an error if reading the file fails', async () => {
+    const readPath = 'config/testInput.yaml';
+    const writePath = 'config/testOutput.json';
+    const error = new Error('File read error');
+
+    fs.readFile.mockRejectedValue(error);
+    console.log = jest.fn();
+
+    await writeJSON(readPath, writePath);
+
+    expect(console.log).toHaveBeenCalledWith(error);
+  });


Try using try catch block to catch error instead of mocking console.log

anshgoyalevil · 2024-06-01T18:20:25Z

scripts/write-json.js

+    try{
+        const readContent = await readFile(readPath,'utf-8');
+        const jsonContent = convertToJson(readContent);
+
+        await writeFile(writePAth,JSON.stringify(jsonContent));
+    }catch(error){
+        console.log(error)


Instead of catching the error for all three operations, you can use nested try catch or different try catch scopes to better write tests that would allow you know which out of these three operations (readFile, convertToJson, writeFile) has failed.

anshgoyalevil · 2024-06-01T18:23:44Z

In the coverage report, there need to be other scripts as well, so that we can track which script has left, i.e., has 0 coverage.
If I am correct, There is a jest configuration to define the folder to track coverage for. Try adding that

akshatnema

I see that you are using console.log statements inside test to assert the values. Instead, you can refactor the functions in such a way that they return some response based on execution, and you can then test their functionalities.

akshatnema · 2024-06-02T15:15:55Z

scripts/write-json.js

Make a util directory inside scripts and add these functions inside this folder. Also change the filename to a relevant one.

akshatnema · 2024-06-02T15:17:15Z

tests/utils.test.js

+
+describe('convertToJson', () => {
+  test('should return JSON object if input is valid JSON string', () => {
+    const jsonString = '{"name": "AsyncAPI", "age": 5}';


you can create fixtures to store the static data that are needed in tests.

anshgoyalevil · 2024-06-11T13:27:32Z

@vishvamsinh28 Any updates on this PR?

vishvamsinh28 · 2024-06-11T13:55:19Z

@anshgoyalevil updated the tests

anshgoyalevil · 2024-06-13T15:13:43Z

tests/utils.test.js

+    try {
+    expect(convertToJson(jsonString)).toEqual(jsonObject);
+    } catch (err) {
+      console.error(err);


add throw statements here also

tests/utils.test.js

anshgoyalevil · 2024-06-16T04:59:09Z

@vishvamsinh28 I have updated the utils.test.js. You can watch out for where exactly the expect statements for test cases related to errors should go.

And, for test cases which are not supposed to fail, they shouldn't ideally be enclosed in a try catch block since the function being tested upon there is already enclosed in a try catch block, which is sufficient to throw an error and fail that specific test case.

For example,

In

test('should return JSON object if input is valid JSON string', () => {
    expect(convertToJson(jsonString)).toEqual(jsonObject);
  });

The convertToJson already contains a try catch block with throw new Error statement. Now, suppose, the function fails to parse the jsonString, the above test case would fail (and it should fail, that what tests are supposed to do) since an error would be thrown in convertToJson.

Update other tests whereever applicable, and make sure there is a throw new Error statement along with console.error statement in the scripts. Similarly, make sure there is no console.error or throw new Error statement inside the tests.

Another thing is the prettier. Don't know why .js files are getting formatted automatically with npm run lint:fix. Maybe a configuration error on eslint side.
For now, you make sure the files changed in this PR are formatted. (Tip: I just press ctrl+s inside a specific file in vscode, and it gets formatted, maybe due to prettier extension?)

akshatnema · 2024-06-06T14:02:22Z

scripts/utils/write-json.js

Can you have a different filename here as per camelCase naming convention and a more meaningful one?

yes will update it

akshatnema · 2024-06-17T06:58:34Z

tests/adopters/index.test.js

+  beforeEach(() => {
+    jest.clearAllMocks();
+  });


Is it required here?

not really but good to have if we add more tests to it

If it will be needed, we'll add it in the future. For now, you can remove it.

akshatnema · 2024-06-17T08:34:00Z

tests/write-json.test.js

+    const error = new Error('File read error');
+
+    fs.readFile.mockRejectedValue(error);
+    const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});


Why do we need to mockImplementation for console.error function?

to prevent the output to log in the console during tests. If we log errors to the console that can clutter the output and make it harder to identify test failures when we have multiple tests

I don't think there will be any problem with console error messages. You shouldn't mock this implementation as it will help us to detect the flow of the function, on a desired test.

Okay 👍 I'll update it

akshatnema · 2024-06-17T08:35:21Z

tests/write-json.test.js

+      `Error reading file from path ${readPath}:`,
+      error
+    );
+    consoleErrorSpy.mockRestore();


Do we need this as we already cleared the mocks in beforeEach callback?

yes we already have it in beforeEach and it works well even if i remove it but clearAllMocks does not restore it to its original implementations so i added it

akshatnema · 2024-06-17T08:41:05Z

tests/utils.test.js

+
+  test('should throw an error if input is invalid JSON and invalid YAML', () => {
+    try {
+      convertToJson(invalidString);


Also check that there shouldn't be any value returned from the function.

tests/write-json.test.js

vishvamsinh28 · 2024-06-18T06:12:32Z

I have updated the tests for throwing errors correctly i 2 have implementations for it which one should push changes for ?

i have updated script as well

and i have removed console.error from the script

@anshgoyalevil @sambhavgupta0705 @akshatnema

vishvamsinh28 · 2024-06-18T06:19:44Z

Another thing is the prettier. Don't know why .js files are getting formatted automatically with npm run lint:fix. Maybe a configuration error on eslint side. For now, you make sure the files changed in this PR are formatted. (Tip: I just press ctrl+s inside a specific file in vscode, and it gets formatted, maybe due to prettier extension?)

i just removed prettier when we added it back to the project because it was giving me a lot of lint/prettier errors 😅
@anshgoyalevil

anshgoyalevil · 2024-06-29T04:16:18Z

@vishvamsinh28 Kindly update PR title, and resolve merge conflicts

anshgoyalevil · 2024-07-01T13:55:57Z

Merging since it already got Akshat's approval 🚢

anshgoyalevil · 2024-07-01T13:56:07Z

/rtm

vishvamsinh28 added 2 commits June 1, 2024 00:27

refactored scripts, added jest and wrote test for it

9749110

added coverage to gitignore

4928ac6

vishvamsinh28 requested review from derberg, akshatnema, magicmatatjahu, anshgoyalevil, Mayaleeeee and asyncapi-bot-eve as code owners May 31, 2024 19:04

asyncapi-bot requested a review from sambhavgupta0705 May 31, 2024 19:05

fix package-lock

cd3a6cc

anshgoyalevil reviewed Jun 1, 2024

View reviewed changes

akshatnema reviewed Jun 2, 2024

View reviewed changes

vishvamsinh28 added 3 commits June 5, 2024 22:55

write json and test updated

42ce7d7

utils test updated

8bf5af9

write json tests updated and fixtures added

3001124

asyncapi-bot requested review from BhaswatiRoy, J0SAL, TRohit20 and VaishnaviNandakumar June 5, 2024 17:47

vishvamsinh28 and others added 2 commits June 5, 2024 23:18

Merge branch 'master' into jestTests

b90f0bf

build fix

7a2a340

test updated

4468fc5

anshgoyalevil reviewed Jun 13, 2024

View reviewed changes

tests/utils.test.js Outdated Show resolved Hide resolved

fix utils.test.js

b27a095

asyncapi-bot requested a review from TRohit20 June 16, 2024 04:54

anshgoyalevil added 2 commits June 16, 2024 10:36

Merge branch 'master' into jestTests

c818765

Update tests/utils.test.js

6673681

akshatnema requested changes Jun 17, 2024

View reviewed changes

name change

afd7eeb

updated tests for errors

af63f97

anshgoyalevil previously approved these changes Jun 20, 2024

View reviewed changes

Merge branch 'master' into jestTests

3bfc5ed

vishvamsinh28 changed the title ~~feat: add jest to the project and refactor scripts~~ feat: write tests and refactor scripts Jul 1, 2024

Merge branch 'master' into jestTests

51522e6

akshatnema dismissed anshgoyalevil’s stale review via 51522e6 July 1, 2024 04:19

reverted package-lock.json

57cc6a1

akshatnema previously approved these changes Jul 1, 2024

View reviewed changes

fix package.json

02faf33

anshgoyalevil dismissed akshatnema’s stale review via 02faf33 July 1, 2024 13:53

anshgoyalevil approved these changes Jul 1, 2024

View reviewed changes

asyncapi-bot added the ready-to-merge label Jul 1, 2024

asyncapi-bot merged commit 09d1652 into asyncapi:master Jul 1, 2024
15 checks passed

feat: write tests and refactor scripts #3012

feat: write tests and refactor scripts #3012

Conversation

vishvamsinh28 commented May 31, 2024

netlify bot commented May 31, 2024 • edited Loading

✅ Deploy Preview for asyncapi-website ready!

asyncapi-bot commented May 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anshgoyalevil commented Jun 1, 2024

akshatnema left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anshgoyalevil commented Jun 11, 2024

vishvamsinh28 commented Jun 11, 2024

Choose a reason for hiding this comment

anshgoyalevil commented Jun 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vishvamsinh28 commented Jun 18, 2024 • edited Loading

vishvamsinh28 commented Jun 18, 2024 • edited Loading

anshgoyalevil commented Jun 29, 2024

anshgoyalevil commented Jul 1, 2024

anshgoyalevil commented Jul 1, 2024

netlify bot commented May 31, 2024 •

edited

Loading

asyncapi-bot commented May 31, 2024 •

edited

Loading

anshgoyalevil commented Jun 16, 2024 •

edited

Loading

vishvamsinh28 commented Jun 18, 2024 •

edited

Loading

vishvamsinh28 commented Jun 18, 2024 •

edited

Loading