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

Reducing boilerplate for tests #117

Merged
merged 7 commits into from
Aug 18, 2020
Merged

Reducing boilerplate for tests #117

merged 7 commits into from
Aug 18, 2020

Conversation

marioortizmanero
Copy link
Collaborator

@marioortizmanero marioortizmanero commented Aug 16, 2020

test_device: Not sure what that's for. Can it be removed?
test_with_credential: Just a function to reduce the boilerplate a bit.
test_oauth: This creates a single spotify client so that it can be used from each test. This way, you don't have to open a browser tab for each test, and there's much less copy-pasted code. This is specially useful in case we figure out how to run these tests on Travis, because it'll be very easy to change the authentication process.

@marioortizmanero marioortizmanero marked this pull request as draft August 16, 2020 17:11
@marioortizmanero marioortizmanero marked this pull request as ready for review August 16, 2020 17:37
@marioortizmanero marioortizmanero changed the title Reducing boilerplate for oauth tests Reducing boilerplate for tests Aug 16, 2020
@marioortizmanero marioortizmanero force-pushed the oauth-tests-boilerplate branch from ac2bf7f to 7a9cfb8 Compare August 16, 2020 17:59
@ramsayleung
Copy link
Owner

test_device: Not sure what that's for. Can it be removed?

Yes, of course, it's useless now.

This creates a single spotify client so that it can be used from each test. This way, you don't have to open a browser tab for each test, and there's much less copy-pasted code. This is specially useful in case we figure out how to run these tests on Travis, because it'll be very easy to change the authentication process.

Yes, we don't need so much copy-pasted code, it will be more clear to remove them, good to see that.

I prefer to keep this PR on until the Spotify Offical responses to our request about applying for a premium account for the library

@marioortizmanero
Copy link
Collaborator Author

Yes, of course, it's useless now.

Okay, I've removed it.

I prefer to keep this PR on until the Spotify Offical responses to our request about applying for a premium account for the library

They've flagged my post as spam so I don't think we can get that premium account. If I manage to do so, I'll let you know. But you can merge for now.

@ramsayleung ramsayleung merged commit b63cc34 into master Aug 18, 2020
@ramsayleung ramsayleung deleted the oauth-tests-boilerplate branch August 18, 2020 01:15
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