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

More unit test coverage #124

Closed
kate-goldenring opened this issue Oct 5, 2023 · 2 comments · Fixed by #127
Closed

More unit test coverage #124

kate-goldenring opened this issue Oct 5, 2023 · 2 comments · Fixed by #127

Comments

@kate-goldenring
Copy link
Collaborator

This repository has very few unit tests. Ideally, adding a command and new api calls would come with a suite of tests. If we create a trait for the cloud Client, we can use mockall to mock api calls and ensure our commands are sending and handling data as expected.

@kate-goldenring
Copy link
Collaborator Author

@itowlson any thoughts on this approach?

@itowlson
Copy link
Contributor

itowlson commented Oct 5, 2023

Yes, this sounds like pretty much the only way, although we'll also have to worry about interactions - some of the interactive code is quite entwined with the integration code. It may be best to do it incrementally to assess how much value it's giving us - my experience of mock/fake-based testing of very integration-heavy code is it can end up as "well we can confirm it makes this series of API calls but 🤷" - hopefully you've had more success with it than I have? - but a few experiments on the hairier bits of the code will give us a sense for that. Otherwise we might be better off figuring out how to do integration tests in a reliable and not-too-slow way...!

In terms of the technology, I tend to prefer ad hoc fakes. I've never met a mocking library I didn't hate with the burning fiery passion of a thousand suns, but that may be down to personal taste (or bad luck with libraries). It's an implementation detail, though, and we can try either approach and change if we're not loving it.

In any event, it would be great to do this - even if we end up struggling with the tests themselves, I'm confident it will drive improvements to the structuring of the code and keep us honest going forward.

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 a pull request may close this issue.

2 participants