-
Notifications
You must be signed in to change notification settings - Fork 988
Unit Tests for API endpoints #434
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
Mmmh yeah, good catch. I've been meaning to take over the code coverage task too sometime, would make it easier to spot the glaring omissions. |
I can help, but need time to figure out how to. Any instruction is appreciated if my help is welcomed. |
Hey guys, I actually have been looking at ways to get involved in some way in the project and was thinking potentially I could learn more rust and more about grin by helping with the testing. I reckon we all probably agree that making sure the test frameworks are stable and repeatable will allow the project to pump ahead with fewer bumps. To that end I reckon it's important for any project to agree on what we mean by unit tests and what we want from the tests in general. Adhering to the terminology here and quick view of the current codebase: https://doc.rust-lang.org/book/second-edition/ch11-03-test-organization.html There are patches of unit tests in some places but 0 unit test in the API. There are also no integration tests for the API. I'd also argue there is room to add another layer of acceptance tests by which I mean an automated framework to spin up a few nodes (likely in docker containers for repeatability) and simulate some common use cases including querying the APIs for state. Sorry for the ramble but I guess my view is first it might be worth documenting what the goals for testing are. For example unit test coverage > 90%, integration tests to cover all main use cases potential error cases and finally a small number of acceptance tests on top of the lot. I'm happy to take a first stab at documenting this this weekend and even do some work to start measuring things like coverage if the approach is agreed. |
Thanks for adding the direct link on testing. Ask on Gitter if you guys run into any problems, setup or "devops" things along the way. Automation that spins up several nodes is out there, but maybe not in this repo and/or not documented. You can open separate issues about that. More testing-relate issues are open, but this one is I think both quite easy and urgent, so a good place to start. |
@edbo we mostly adhere to the conventions you linked to. And testing is extremely valuable. But there's some nomenclature that should be clarified:
So one can't do unit tests on the API. But there should be extensive integration tests. And the same should be true for many other areas of the codebase. And coverage would be a great place to start. Once we have an idea of what coverage looks like right now, we can decide what are the best areas to concentrate on and gradually get coverage up. |
Cool I made a small start trying to get some coverage on the unit tests. I'll try and get it working and push in some info about unit testing as that seems on a part we can all agree on first. You guys OK with using this: https://github.com/codecov/example-rust It will be free for this public repo and they have more obvious examples being used with Rust. |
kvoc and that shiny badge? Yes please! Travis is in use already. If configuration on https://travis-ci.org/mimblewimble/ need to be updated, probably ask @ignopeverell |
@ignopeverell Thoughts? Mind if I bring in a dependency on cargo-make so I can try and tie together the various test runs in the directories? |
Go ahead :) we really want tests. Also see stalled attempt in PR #115. Commiters are hungry for better test coverage overview, and once we measure coverage, we'll be better incentivised to add tests. |
Yes, I'm all for it. |
Just leaving a link; https://github.com/steveklabnik/quickcheck can be used to create |
Uh oh!
There was an error while loading. Please reload this page.
The recent update to API endpoints here #416 appear to have broken wallet restore; that's completely forgivable. What isn't forgivable is that something like this should have been easily caught in automated testing, which I don't think we do at all for the API (unless I'm mistaken) or if we do anywhere, they're very out of date.
Unit tests should be put into place that cover the API endpoints. Should be simple enough, build up a small chain and a wallet in a cargo test, then put a few tests together to make sure all the endpoints return more or less what's expected.
The text was updated successfully, but these errors were encountered: