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

adding APIs for app test user. #172

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

theeluwin
Copy link

This commit is sample implementation of #96.
Since TestGetAppAccessToken from test/test_facebook.py does not guarantees a valid app access token, how should I write meaningful test codes?

@@ -293,6 +293,38 @@ def get_app_access_token(self, app_id, app_secret):

return self.request("oauth/access_token", args=args)["access_token"]

def create_app_test_user(self, app_id, app_access_token, **kwargs):
Copy link
Member

Choose a reason for hiding this comment

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

I think it might be simpler and more consistent if all methods were called *_test_user (i.e. create_test_user, edit_test_user, get_test_users, etc.).

@martey
Copy link
Member

martey commented Oct 25, 2014

Since TestGetAppAccessToken from test/test_facebook.py does not guarantees a valid app access token, how should I write meaningful test codes?

TestGetAppAccessToken does not explicitly validate the app access token to ensure that it is valid. If get_app_access_token is returning invalid access tokens, TestFQL will also fail.

I am not sure some of the docstrings are actually useful (e.g. it's obvious that the delete_test_user method deletes a test user) and some of the information looks wrong (the methods will return the response from the GraphAPI if they succeed, not True).

@martey martey added this to the v1.0.0 milestone Oct 27, 2014
@martey martey mentioned this pull request Oct 27, 2014
@martey
Copy link
Member

martey commented Feb 22, 2016

In order to release 1.0.0 as soon as possible, I am moving this to the 2.0.0 milestone. This isn't a reflection of the quality of your pull request, but of the lack of free time I currently have to devote to this project.

@martey martey modified the milestones: v2.0.0, v1.0.0 Feb 22, 2016
@theeluwin
Copy link
Author

Got it. Thanks. But actually, I still feel sorry for quite poor quality of my PR, while still not sure how to improve it.

@martey martey modified the milestones: v3.0.0, v2.0.0 Aug 8, 2016
@martey martey mentioned this pull request Oct 14, 2017
@martey martey modified the milestones: v3.0.0, v3.1.0 Jul 19, 2018
@martey martey modified the milestones: v3.1.0, v3.2.0 Nov 6, 2018
@martey martey modified the milestones: v3.2.0, v5.0.0 Jul 28, 2020
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