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

Fixed code and unit tests #645

Merged
merged 15 commits into from
Aug 1, 2023
Merged

Fixed code and unit tests #645

merged 15 commits into from
Aug 1, 2023

Conversation

jbigman
Copy link
Contributor

@jbigman jbigman commented Jul 26, 2023

  • fixed search: more section title is not always empty, it can contain "Apps"
  • fixed missing comments (ds:9 => ds:8)
  • new url called to test throttle,
  • updated expected results

Copy link
Owner

@facundoolano facundoolano left a comment

Choose a reason for hiding this comment

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

Lgtm except for those logs, remove and I'll merge

return gplay.search({ term: 'netflix' })
.then((apps) => {
console.log(apps.map((app) => app.appId));
Copy link
Owner

Choose a reason for hiding this comment

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

Don't leave console logs in the code

});
});

it('should return few netflix apps from german store with german language', () => {
return gplay.search({ term: 'netflix', lang: 'de', country: 'DE' })
.then((apps) => {
console.log(apps.map((app) => app.appId));
Copy link
Owner

Choose a reason for hiding this comment

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

Same

@jbigman
Copy link
Contributor Author

jbigman commented Jul 28, 2023

I don't know why the throttle tests are timing out here, the build passed at my fork.
https://github.com/jbigman/google-play-scraper/actions/runs/5692222528

@facundoolano
Copy link
Owner

Seems to fail consistently... I'll merge this anyway as it's an improvement from the current code, but we should try to figure out why it doesn't pass.

@facundoolano facundoolano merged commit b9df4e0 into facundoolano:main Aug 1, 2023
2 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