Skip to content

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

Closed
yeastplume opened this issue Dec 6, 2017 · 11 comments
Closed

Unit Tests for API endpoints #434

yeastplume opened this issue Dec 6, 2017 · 11 comments

Comments

@yeastplume
Copy link
Member

yeastplume commented Dec 6, 2017

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.

@ignopeverell
Copy link
Contributor

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.

@heunglee
Copy link
Contributor

heunglee commented Dec 6, 2017

I can help, but need time to figure out how to. Any instruction is appreciated if my help is welcomed.

@edbo
Copy link

edbo commented Dec 8, 2017

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.

@sesam
Copy link
Contributor

sesam commented Dec 8, 2017

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.

@ignopeverell
Copy link
Contributor

@edbo we mostly adhere to the conventions you linked to. And testing is extremely valuable. But there's some nomenclature that should be clarified:

  • Unit tests can be run in isolation. No i/o is involved, so no file system access, no storage, no network, http or tcp server. Only span a single crate/module.
  • Integration tests are the opposite. All the messiness of i/o plus multiple moving parts.

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.

@edbo
Copy link

edbo commented Dec 12, 2017

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.

@sesam
Copy link
Contributor

sesam commented Dec 12, 2017

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

@edbo
Copy link

edbo commented Dec 12, 2017

@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?

@sesam
Copy link
Contributor

sesam commented Dec 12, 2017

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.

@ignopeverell
Copy link
Contributor

Yes, I'm all for it.

@sesam
Copy link
Contributor

sesam commented Dec 13, 2017

Just leaving a link; https://github.com/steveklabnik/quickcheck can be used to create cargo test fuzzing tests = feed in random inputs and find a small, maybe minimal, example where the function fails. We should use this for network-facing functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants