-
Notifications
You must be signed in to change notification settings - Fork 8
docs: add Admin API samples for account management methods #65
Conversation
…ics-admin into samples_accounts
Here is the summary of changes. You are about to add 10 region tags.
This comment is generated by snippet-bot.
|
Region tag product prefix check is failing due to the product set as non-public in https://devrel.corp.google.com/admin/products/401. This is something that we are currently working on, but I believe the region prefixes are fine. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I stopped about halfway through because it was going to be obnoxious to see me say "same as above" on the tests - the samples themselves look great as far as I can tell, and the tests aren't bad as is (they're independent of each other and valid! Woo!), but I think they could be hardened with the use of fixtures. If you need it, this cloud storage test has some examples of function scoped fixtures and crud operations
def test_accounts_user_links_audit(capsys): | ||
accounts_user_links_audit.audit_account_user_links(TEST_ACCOUNT_ID) | ||
out, _ = capsys.readouterr() | ||
assert "Result" in out |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would probably make sense to add a few more asserts for your other print statements just to be super sure they happen
TEST_EMAIL_ADDRESS = os.getenv("GA_TEST_EMAIL_ADDRESS") | ||
|
||
|
||
def test_accounts_user_links_batch_create(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to test the happy path and delete it afterwards with a fixture? I do like the creative 403 as an additional test
"""Runs the sample.""" | ||
|
||
# !!! ATTENTION !!! | ||
# Running this sample may change/delete your Google Analytics account |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
love this very helpful comment A+ DevX it caught my attention immediately
FAKE_ACCOUNT_USER_LINK_ID = "1" | ||
|
||
|
||
def test_accounts_user_links_batch_delete(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
similar to the create, would love to see a test that uses a fixture to create then actually deletes and checks to see that the value is no longer there
TEST_ACCOUNT_ID, TEST_USER_LINK_ID | ||
) | ||
out, _ = capsys.readouterr() | ||
assert "Result" in out |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the value you're expecting known? Could you add more to this assert if it's the same test value you're looking for every time?
(Should you add a value in a fixture that you then "get" in the test and delete in the teardown?)
FAKE_ACCOUNT_USER_LINK_ID = "1" | ||
|
||
|
||
def test_accounts_user_links_batch_update(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as others - would love to see a fake account that gets created and deleted in a fixture and updated in the test
Leah, to address comments above. Problem is, it is currently impossible to create a new Google Analytics account programmatically using a public API due to the need for a user to accept Terms of Services. The feature to provision a new account automatically will likely be implemented by end of Q3, so we might be able to do more interesting tests. Right now, I would prefer to not mutate the state of the test GA account since there is no easy way to reset it to the original state in case something goes wrong. As a result, the tests mainly check for the validity of the request proto for now. |
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕