Skip to content
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

Fix variable number of reviews returned #676 #677

Merged
merged 5 commits into from
Apr 9, 2024

Conversation

petskratt
Copy link
Contributor

According to issue #676 reviews (and presumably other listings) that require pagination are returning inconsistent results. Implementing cookieJar appears to fix that.

Not tested with other functions.

@petskratt
Copy link
Contributor Author

All fails appear to be apps that have been removed from store - air.com.zinkia.playset (app method), air.tv.ingames.cubematch.free and com.skybornegames.battlepop (permisssions method).

@facundoolano
Copy link
Owner

this LGTM. perhaps you can extend the pagination related test to check that the same query returns the same results every time (ideally this would be failing in main and would be fixed by this change)

For detection of facundoolano#676 (less than actual number of requests returned, different response count on each request).

Using app with large number of reviews sorted by helpfulness (assuming this set does not change as frequently as just reviews), num: 1500 is reasonably fast and we get 2 x 10 paged requests (of 150 results) that ought to prove paging works.
@petskratt
Copy link
Contributor Author

Good point - added test that utilises built-in paging, fails on main and works with cookies. As several other tests fail due to discontinued apps ... com.facebook.katana will hopefully stay up for some time and considering the number of reviews perhaps the set of 1500 most helpful ones does not change too often.

@facundoolano facundoolano merged commit c0bdcec into facundoolano:main Apr 9, 2024
0 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants