-
-
Notifications
You must be signed in to change notification settings - Fork 303
test: add tests for bindings.js#getQueryParams #1576
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.
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
achaljhawar
wants to merge
22
commits into
asyncapi:master
Choose a base branch
from
achaljhawar:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
e2b4f5d
test: bindings.js#getQueryParams
achaljhawar e36c05a
fix: undefined channel.bindings() bug
achaljhawar be65fb2
fix: resolve ESLint violations in bindings test
achaljhawar 333433d
refactor: created a TEST_CHANNEL_NAME variable
achaljhawar 546a041
Merge branch 'master' into master
achaljhawar 3f31e79
test: use test fixtures in bindings.test.js
achaljhawar f0fbbcc
Merge branch 'master' into master
achaljhawar 196311d
Merge branch 'master' into master
achaljhawar 024e954
test: fixed getQueryParams tests by removing mocks
achaljhawar b61121a
Merge branch 'master' into master
achaljhawar ee08acc
Merge branch 'master' into master
achaljhawar e69f4b1
refactor: use Map.delete() instead of filtering for test channels
achaljhawar 16c0f42
Merge branch 'master' into master
achaljhawar 2011e46
Merge branch 'master' into master
achaljhawar 6bdaf8f
refactor: Simplify createChannelsWithOnly in bindings.test.js
achaljhawar bda84bc
Merge branch 'master' of https://github.com/achaljhawar/generator
achaljhawar b89c84d
Merge branch 'master' into master
achaljhawar d1b4adc
refactor: changed import for getQueryParams
achaljhawar 5a09394
Merge branch 'master' of https://github.com/achaljhawar/generator
achaljhawar bb9a194
Merge branch 'master' into master
achaljhawar 7d9eb64
refactor: eliminated code duplication in bindings.test.js
achaljhawar 73aa16a
docs: add comments explaining mocking and Map copying in bindings tests
achaljhawar File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
const path = require('path'); | ||
const { Parser, fromFile } = require('@asyncapi/parser'); | ||
const { getQueryParams } = require('@asyncapi/generator-helpers'); | ||
const parser = new Parser(); | ||
const asyncapi_v3_path = path.resolve(__dirname, './__fixtures__/asyncapi-websocket-query.yml'); | ||
|
||
function createChannelsWithOnly(keysToKeep, originalChannels) { | ||
// Create a copy to avoid mutating the original channels map - | ||
// other tests need the original data intact | ||
const channelsMap = new Map(originalChannels.all()); | ||
for (const key of channelsMap.keys()) { | ||
if (!keysToKeep.includes(key)) { | ||
channelsMap.delete(key); | ||
} | ||
} | ||
return channelsMap; | ||
} | ||
|
||
describe('getQueryParams integration test with AsyncAPI', () => { | ||
let parsedAsyncAPIDocument; | ||
|
||
beforeAll(async () => { | ||
const parseResult = await fromFile(parser, asyncapi_v3_path).parse(); | ||
parsedAsyncAPIDocument = parseResult.document; | ||
}); | ||
|
||
// Helper function to create filtered channels and get query params | ||
const getQueryParamsForChannels = (channelNames) => { | ||
const channels = parsedAsyncAPIDocument.channels(); | ||
const filteredChannelsMap = createChannelsWithOnly(channelNames, channels); | ||
// Mock the channels interface that getQueryParams() expects - it needs | ||
// isEmpty() and all() methods, not direct Map access | ||
return getQueryParams({ | ||
isEmpty: () => filteredChannelsMap.size === 0, | ||
all: () => filteredChannelsMap | ||
}); | ||
}; | ||
|
||
it('should extract query parameters from WebSocket binding with properties', () => { | ||
const channels = parsedAsyncAPIDocument.channels(); | ||
const params = getQueryParams(channels); | ||
|
||
expect(params).not.toBeNull(); | ||
expect(params.get('heartbeat')).toBe('false'); | ||
expect(params.get('top_of_book')).toBe('false'); | ||
expect(params.get('bids')).toBe('true'); | ||
expect(params.get('offers')).toBe(''); | ||
}); | ||
|
||
it('should return null for channel without WebSocket binding', () => { | ||
const params = getQueryParamsForChannels(['marketDataV1NoBinding']); | ||
expect(params).toBeNull(); | ||
}); | ||
|
||
it('should return null for empty channels map', () => { | ||
// Mock channels object to simulate empty state - getQueryParams() expects | ||
// channels with isEmpty() and all() methods, not a plain Map | ||
const emptyChannels = { | ||
isEmpty: () => true, | ||
all: () => new Map() | ||
}; | ||
const params = getQueryParams(emptyChannels); | ||
expect(params).toBeNull(); | ||
}); | ||
|
||
it('should return null for channel with empty binding', () => { | ||
const params = getQueryParamsForChannels(['emptyChannel']); | ||
expect(params).toBeNull(); | ||
}); | ||
|
||
it('should return null if WebSocket binding exists but has no query parameters', () => { | ||
const params = getQueryParamsForChannels(['wsBindingNoQuery']); | ||
expect(params).toBeNull(); | ||
}); | ||
|
||
it('should return null if WebSocket binding query exists but has no properties', () => { | ||
const params = getQueryParamsForChannels(['wsBindingEmptyQuery']); | ||
expect(params).toBeNull(); | ||
}); | ||
}); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why we need to create a new map here, and still do some mocking basically of
all
andisEmpty
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey are you still working on something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am done ig
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ohk I'll fix this
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should I create a second fixture file to fix this problem?
or should I test with the full channel object and verify its behaviour?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's not decide yet, first answer my question 😄 why new map is needed, why you cannot operate in the helper on the map you get as input
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new Map is needed to create a copy so the original channels map isn't mutated, which would affect other tests that need the data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, now much better. Remember reviewer did not code and do not see what issues you had during development. Sometimes we just ask question not to play smart, but to really get an answer 😄 thanks for the context - now it makes sense why you've done so, and why later you still do mocking of
all
andisEmpty
- please add comment to tested code to explain why this mocking is needed