-
Notifications
You must be signed in to change notification settings - Fork 6
Add unit tests #5
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
Comments
We can either do unit tests against a mock API, or use do it against a prod Listenbrainz account. I would do both personally - for the security of the token, my suggestion is to: 1) use an environment variable for the token to be accessed by the unit tests and 2) use GitHub secrets in Actions for setting the token as an environment variable. |
Sounds good. Feel free to take charge with this if you want, I have no experience with testing HTTP APIs myself. You have access to this repository's secrets, right? |
We can do mock APIs, or test against prod. The problem with the latter is that it will require internet access. Thoughts? No, I don't have access to settings on this repo at the moment. |
We should definitely test against a mock API. Not sure if testing against prod is too useful on second thought, and I doubt spinning up a full custom ListenBrainz instance is viable. Strange, I thought all collaborators had full access. |
Yeah, prod would also be an issue if there's multiple CI builds running at the same time. Well, I can't see a settings tab... |
I think I misread the Secrets section of the settings, if I add a secret you will be able to use it but you can't add secrets yourself. Doesn't matter anyway if we don't test against the live API. |
@shymega just checking in to see if you've had a chance to look at this yet? Totally fine if not, just curious 😄 I would like to release the new version soon (ish) to use it in |
Pushing this back to a future version so |
Hey sorry, I've been really busy. I'm trying to catch up at the moment, but I'm starting a course on Monday, and then work placements for the rest of the week, so I'm not sure how much time I'll have... I do feel bad you've been left to do a lot of the work here. I'll try and catch up soon. Sorry. |
No worries, thanks for the update! Take all the time you need. |
No description provided.
The text was updated successfully, but these errors were encountered: