Skip to content

test: fix shared mock state and improve test isolation #2023

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
SHUBHANSHU602 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

test: fix shared mock state and improve test isolation #2023

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions apps/generator/test/generator.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
/* eslint-disable sonarjs/no-duplicate-string */
jest.mock('@asyncapi/parser', () => ({
convertToOldAPI: jest.fn((doc) => doc),
}));
Comment on lines +2 to +4
Copy link
Copy Markdown
Member

@Adi-204 Adi-204 Mar 7, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why we need this change?

Copy link
Copy Markdown
Contributor Author

@SHUBHANSHU602 SHUBHANSHU602 Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Adi-204 I know it seems like voilating the scope, BUT

While fixing the isolation issue, the tests started reaching the actual parser conversion logic inside lib/parser.js. Previously, because of shared mutable state in the mocks and the way tests were structured, this code path was not always executed consistently.

When the tests began running in a properly isolated state, they attempted to call convertToOldAPI from @asyncapi/parser. However, newer versions of @asyncapi/parser no longer expose this function, which results in the runtime error:

TypeError: convertToOldAPI is not a function

Since the goal of this test suite is to verify the behavior of the Generator class rather than the internals of the parser library, I mocked convertToOldAPI to return the document unchanged. This keeps the tests focused on generator behavior while avoiding failures caused by parser implementation details.

If you prefer, I can also adjust the tests to avoid mocking the parser and instead rely on the actual parser behavior.

Copy link
Copy Markdown
Member

@Adi-204 Adi-204 Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @SHUBHANSHU602, I tried removing the mock locally and ran the tests, and they all passed without any TypeError. So I’m not able to reproduce the issue on my end. Because of that, I’m not sure how the tests are reaching the parser conversion logic in lib/parser.js. Could you share a bit more detail on how you observed this behavior? That would help me better understand the issue and verify it properly.

Copy link
Copy Markdown
Contributor Author

@SHUBHANSHU602 SHUBHANSHU602 Mar 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Adi-204 I think I have made the things quite complicated , I will rewrite all the things , to keep the things simple and to avoid scope voilation and confusion.

const fs = require('fs');
const path = require('path');
const Generator = require('../lib/generator');
Expand All @@ -13,6 +16,17 @@ jest.mock('../lib/hooksRegistry');
jest.mock('../lib/templates/config/validator');
jest.mock('../lib/templates/config/loader');

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

const utils = require('../lib/utils');

utils.__files = {};
utils.__contentOfFetchedFile = '';
utils.__isFileSystemPathValue = false;
utils.__generatorVersion = '';
utils.__getTemplateDetails = undefined;
});
describe('Generator', () => {
describe('constructor', () => {
it('works with minimum amount of params', () => {
Expand Down Expand Up @@ -342,9 +356,7 @@ describe('Generator', () => {
const templatePath = './testTemplate';
const gen = new Generator(templatePath, __dirname);
await gen.installTemplate();
setTimeout(() => { // This puts the call at the end of the Node.js event loop queue.
expect(arboristMock.reify).toHaveBeenCalledTimes(0);
}, 0);
expect(arboristMock.reify).not.toHaveBeenCalled();
});

it('works with a file system path and force = true', async () => {
Expand All @@ -367,9 +379,7 @@ describe('Generator', () => {
utils.__isFileSystemPathValue = false;
const gen = new Generator('nameOfTestTemplate', __dirname);
await gen.installTemplate();
setTimeout(() => { // This puts the call at the end of the Node.js event loop queue.
expect(arboristMock.reify).toHaveBeenCalledTimes(0);
}, 0);
expect(arboristMock.reify).not.toHaveBeenCalled();
});

it('works with an npm package that is installed for the first time', async () => {
Expand Down