Fix digital twins query pagination test #16087
Conversation
This test frequently failed claiming that only one page of results was returned from a query. In order to fix this, the test will now create more than just the minimum amount of queryable twins, and it will wait until at least pageSize + 1 twins can be queried before testing that we can control the page size
timtay-microsoft
requested review from
abhipsaMisra,
azabbasi,
barustum,
bikamani,
drwill-ms and
vinagesh
as code owners
ghost
added
the
| @@ -97,13 +99,35 @@ public async Task Query_PaginationWorks() | |||
| // act | |||
| var options = new QueryOptions(); | |||
| options.MaxItemsPerPage = pageSize; | |||
| AsyncPageable<string> asyncPageableResponse = client.QueryAsync(queryString, options); | |||
|
|
|||
| var queryWaitTimeoutStopwatch = new Stopwatch(); | |||
There was a problem hiding this comment.
fyi you can do Stopwatch.StartNew() to make this and only take 1 statement instead of 2. :)
| queryWaitTimeoutStopwatch.Start(); | ||
| while (!queryHasExpectedCount) | ||
| { | ||
| if (queryWaitTimeoutStopwatch.ElapsedMilliseconds >= QueryWaitTimeoutMillis) |
There was a problem hiding this comment.
Why not use a timed cancellation token source instead?
There was a problem hiding this comment.
Oh, so we keep running the query until it finally returns all the items we expect? There must be a delay in items being created and then showing up in a query. Is that right?
There was a problem hiding this comment.
Because I'm a Java developer, haha...
There was a problem hiding this comment.
I like that idea better for .NET code, though, sure
There was a problem hiding this comment.
Oh, so we keep running the query until it finally returns all the items we expect? There must be a delay in items being created and then showing up in a query. Is that right?
That's my hunch, yes. I've seen this same issue in our query tests in IoT Hub, and this fix stabilized the tests
|
|
||
| AsyncPageable<string> asyncPageableResponse = client.QueryAsync(queryString); | ||
| int count = 0; | ||
| await foreach (string queriedTwin in asyncPageableResponse) |
There was a problem hiding this comment.
If you enumerate pages instead of items, do the pages tell you how many items are in it?
There was a problem hiding this comment.
It does. I can iterate over the pages here and just do count+=page.Values.Count; instead
| throw new AssertionException($"Timed out waiting for at least {pageSize + 1} twins to be queryable"); | ||
| } | ||
|
|
||
| AsyncPageable<string> asyncPageableResponse = client.QueryAsync(queryString); |
There was a problem hiding this comment.
do you want to pass the cancellation token into this or the AsPages methods?
There was a problem hiding this comment.
Sure, just in case
This test frequently failed claiming that only one page of results was returned from a query. In order to fix this, the test will now create more than just the minimum amount of queryable twins, and it will wait until at least pageSize + 1 twins can be queried before testing that we can control the page size
This test frequently failed claiming that only one page of results was returned from a query. In order to fix this, the test will now create more than just the minimum amount of queryable twins, and it will wait until at least pageSize + 1 twins can be queried before testing that we can control the page size