test: fix test for pagination cursor #1138
Conversation
VebjornG
changed the title
|
|
||
| expect((await alerts).length).toBeGreaterThan(1000); | ||
| expect((await alerts).length).toBe(50); |
There was a problem hiding this comment.
I don't understand how this tests pagination. Aren't we just fetching 50 alerts without pagination here?
Also autoPagingToArray({ limit: 50 }); will not provide a limit query param to the API as this is logic for the SDK. If you want to send a limit query param to the API, then it should be here:
const alerts = client.alerts.list({..., limit: 25})`
There was a problem hiding this comment.
Also, this is not a fix as specified in the PR title and commit.
From the end-users point of view, this PR doesn't fix any bugs.
The proper prefix is test
See the contribution docs: https://github.com/cognitedata/cognite-sdk-js/blob/master/CONTRIBUTING.md#making-changes
VebjornG
changed the title
| @@ -244,55 +245,34 @@ describe('alerts api', () => { | |||
| property: 'createdTime', | |||
| order: 'desc', | |||
| }, | |||
| cursor: '', | |||
| }); | |||
| expect(response.items.length).toBe(1); | |||
There was a problem hiding this comment.
No alarm has been created at this point, so why is this expected?
Is it coming from a previous test?
In the previous test of the pagination cursor in alert-api, 1001 alerts was created. This created 504s, so we reduced that number to 50.
Ticket.