Skip to content

Conversation

@Rosuavio
Copy link
Contributor

@Rosuavio Rosuavio commented May 12, 2021

Tests different items per page for IncrementalLoadingCollection

PR Type

What kind of change does this PR introduce?

Tests different values for itemsPerPage in the IncrementalLoadingCollection ctor.

What is the current behavior?

What is the new behavior?

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tested code with current supported SDKs
  • Pull Request has been submitted to the documentation repository instructions. Link:
  • Sample in sample app has been added / updated (for bug fixes / features)
  • New major technical changes in the toolkit have or will be added to the Wiki e.g. build changes, source generators, testing infrastructure, sample creation changes, etc...
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Header has been added to all new source files (run build/UpdateHeaders.bat)
  • Contains NO breaking changes

Other information

@ghost
Copy link

ghost commented May 12, 2021

Thanks RosarioPulella for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

@ghost ghost requested review from Kyaa-dost, azchohfi and michael-hawker May 12, 2021 16:44
@ghost ghost added the bug 🐛 An unexpected issue that highlights incorrect behavior label May 12, 2021
@michael-hawker
Copy link
Member

@RosarioPulella was this just to check tests, should we commit this or are we waiting on something else still?

@Rosuavio
Copy link
Contributor Author

The original intention was to explore IncrementalLoadingCollection.LoadMoreItemsAsync(unit count) and providing different values for its param. But that did not seem to work the way I thought it would so instead this PR just tests the itemPerPage from the IncrementalLoadingCollection ctor.

@Rosuavio Rosuavio marked this pull request as ready for review June 22, 2021 19:07
@Rosuavio Rosuavio changed the title Test and Fix IncrementalLoadingCollection PageSize Test IncrementalLoadingCollection itemsPerPage ctor param Jun 22, 2021
@michael-hawker michael-hawker added this to the 7.1 milestone Jun 24, 2021
@michael-hawker michael-hawker added testing 🏗️ and removed bug 🐛 An unexpected issue that highlights incorrect behavior labels Jun 24, 2021
for (var pageNum = 1; pageNum <= pages; pageNum++)
{
requests.Add(collection.LoadMoreItemsAsync(0).AsTask()
.ContinueWith(t => Assert.IsTrue(t.IsCompletedSuccessfully)));
Copy link
Member

Choose a reason for hiding this comment

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

Should we check the interim results for pageSize here as well as we're passing that into the test now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure its that easy since these calls are happening asynchronously and we would need to test ageist collection which will be changing as these tasks complete.

@michael-hawker michael-hawker added the for-review 📖 To evaluate and validate the Issues or PR label Jul 15, 2021
@michael-hawker michael-hawker merged commit 5e4b0b8 into CommunityToolkit:main Aug 30, 2021
@Rosuavio Rosuavio deleted the ilc-pageSize branch August 30, 2021 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

for-review 📖 To evaluate and validate the Issues or PR testing 🏗️

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants