-
Notifications
You must be signed in to change notification settings - Fork 82
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
integration tests #531
integration tests #531
Conversation
LGTM mostly, but I had some failures running on my personal test account in record fixture mode. I think it might be better to make these assertions more flexible
|
I have test on my personal account and GA account as well, passing both cases |
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.
Got some failures:
--- FAIL: TestInvoice_Get (0.00s)
--- FAIL: TestInvoiceItems_List (0.00s)
--- FAIL: TestAccountSettings_Get (0.00s)
--- FAIL: TestAccountSettings_Update (0.00s)
--- FAIL: TestAccountTransfer_Get (0.00s)
Looks like you'll need a quick |
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.
LGTM, tests pass locally. Nice work!
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.
Tests are passing on my end, great work!
closing this PR and raising the new PR due the base branch requires all commits to be signed resolving it creating too many conflicts |
📝 Description
Added Integration Test Account modules
What does this PR do and why is this change necessary?
Test coverage
✔️ How to Test
make testint
📷 Preview
If applicable, include a screenshot or code snippet of this change. Otherwise, please remove this section.