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

LB-148: Add integration tests to api_compat feature #355

Merged
merged 2 commits into from Feb 17, 2018

Conversation

kartikeyaSh
Copy link
Contributor

Summary

This PR includes some tests for api_compat feature

  • This is a…
    • Bug fix
    • Feature addition
    • Refactoring
    • Minor / simple change (like a typo)
    • Other
  • Describe this change in 1-2 sentences:

Problem

Currently there are no tests for last.fm compatible API. Which makes it hard to debug and figure out
if it's working or not.

  • JIRA ticket (optional): LB-148

Solution

Add tests.

@kartikeyaSh
Copy link
Contributor Author

It depends on PR for LB-308 and LB-309.

@paramsingh
Copy link
Collaborator

Would it be a lot of work to move relevant tests to their PRs? Perhaps we could merge #353 with just the one test, followed by adding more tests in this PR (or other bugfix PRs)

@kartikeyaSh
Copy link
Contributor Author

Yes, that can be done. Tests in the first 2 commits will pass with current state of API. Should i delete other commits from this PR to make it valid?

@paramsingh
Copy link
Collaborator

Let's try to get #353 merged first and then you can rebase this.

Copy link
Collaborator

@paramsingh paramsingh left a comment

Choose a reason for hiding this comment

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

Looks good to me overall, but I think you should rebase this once over master.

This commit includes the test to check if the token
generated is valid or not and returns appropriate
error codes.
Tests for session establishing using valid token and invalid token
are written in this.
Copy link
Collaborator

@paramsingh paramsingh left a comment

Choose a reason for hiding this comment

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

💯 🥇

@paramsingh paramsingh merged commit 283a2eb into metabrainz:master Feb 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants