Skip to content

Add tests#204

Merged
smaeda-ks merged 4 commits intoxdevplatform:masterfrom
smaeda-ks:smaeda-ks/develop/tests
Jul 23, 2019
Merged

Add tests#204
smaeda-ks merged 4 commits intoxdevplatform:masterfrom
smaeda-ks:smaeda-ks/develop/tests

Conversation

@smaeda-ks
Copy link
Copy Markdown
Contributor

@smaeda-ks smaeda-ks commented Jul 21, 2019

  • Adding Sync and Async Analytics tests (exclude fetching .gz data part for now)
  • Fix Active Entities test 91a8666
  • Fix async_stats_job_result() method in Analytics class 65ea9ef

- fix function name
- add new assert to check request URL (query parameter) to make sure optional parameters are passed as intended
@smaeda-ks smaeda-ks self-assigned this Jul 21, 2019
when:
- there's no resource property for the class (e.g., TargetingCriteria)
- the class is inherited to other class
@smaeda-ks smaeda-ks force-pushed the smaeda-ks/develop/tests branch from 5d94ded to 3d6f410 Compare July 23, 2019 02:01
Comment thread twitter_ads/resource.py
request = Request(account.client, 'get', resource, params=params)

return Cursor(klass, request, init_with=[account])
return Cursor(None, request, init_with=[account])
Copy link
Copy Markdown
Contributor Author

@smaeda-ks smaeda-ks Jul 23, 2019

Choose a reason for hiding this comment

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

Otherwise, this behaves differently when this method is called directly from Analytics class Analytics.async_stats_job_result() vs called through inherited class Campaign.async_stats_job_result(). The later one returns campaign instance and can't access return values as the Campaign class obviously doesn't have resource property for the Analytics endpoint.

https://github.com/twitterdev/twitter-python-ads-sdk/blob/master/twitter_ads/cursor.py#L96-L102

@smaeda-ks smaeda-ks changed the title [WIP] Add tests Add tests Jul 23, 2019
@smaeda-ks smaeda-ks merged commit fe282fb into xdevplatform:master Jul 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants