-
Notifications
You must be signed in to change notification settings - Fork 652
test(client-sqs): check for content-type in input and output #7656
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
base: main
Are you sure you want to change the base?
Conversation
|
|
||
| it(`can make requests with ${client.config.protocol.constructor.name}`, async () => { | ||
| const queues = await client.listQueues(); | ||
| expect(queues.QueueUrls ?? []).toBeInstanceOf(Array); |
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 test should retain this brief assertion of the output JS structure, shallow though it may be
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.
This will always be true, since it's always an instance of Array (even if QueueUrls is undefined)
Welcome to Node.js v20.19.0.
Type ".help" for more information.
> (undefined ?? []) instanceof Array
trueIf there are no queues in the account, QueueUrls is undefined
{
'$metadata': {
httpStatusCode: 200,
requestId: '7652a27f-b8b7-56a0-9887-c39ebdf93168',
extendedRequestId: undefined,
cfId: undefined,
attempts: 1,
totalRetryDelay: 0
}
}
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.
We already have expect.assertions to confirm that assertions inside middleware are called.
I still added check for httpStatusCode in 598e2ed
| }, | ||
| { step: "build" } | ||
| ); | ||
| await client.listQueues({ MaxResults: 1 }); |
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.
| await client.listQueues({ MaxResults: 1 }); | |
| const queues = await client.listQueues({ MaxResults: 1 }); | |
| expect(queues.QueueUrls ?? []).toBeInstanceOf(Array); |
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 added check for httpStatusCode in 598e2ed
Issue
Noticed while examining e2e test output in JS-6477
Description
Checks for content-type in input and output when different protocol is passed.
Also saves time while executing test by listing just one queue.
Testing
Before
Prints lot of debug statements, and doesn't check for content-type used by protocol.
After
Doesn't print debug statements, and checks for content-type used by protocol.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.