tests: better coverage across the board

[jessfraz]

• add integration tests ensuring all the behavior we have outlined in the design spec
• better unit test coverage

Adding the good-first-pr label, because test could be added for any function, big or small, we also have a framework for integration testing where you could easily copy paste another test and change some things to test something else.

Here is an example of an integration test: https://github.com/golang/dep/blob/master/remove_test.go#L9

If you want to write a test, just write here what you wish to test so no one takes the same one :)

[jessfraz]

adding the good first pr label, because test could be added for any function, big or small, we also have a framework for integration testing where you could easily copy paste another test and change some things to test something else

[tro3]

Looks like Context.LoadProject is uncovered. Any particular reason? If not, I can work on a unit test for that one after I figure out how you've all set up / torn down test files for os calls in other tests. Sound okay?

[sdboyer]

@tro3 no reason for it :) that's be a great help - you'd effectively be codifying some very important rules about how we decide what the boundaries of projects. Thanks!

[tro3]

No problem. Do you have a doc as to what the intended behavior is, or would you prefer I use the existing code as the "spec" and write tests to cover it and guard against future breakage?

[sdboyer]

@tro3 nope, no actual spec for it, so best to take code as spec and work from there.

ideally...if you have time 😄 ...you could also write up some docs explaining how it works, so that we can sorta retro-spec it. would ease discussions about it in the future, and/or contribute to the docs that we are sorely lacking

[tro3]

Will do. One question - I assume the lock file is supposed to always be in the same directory as the manifest? If so, there is a bug when locating the root while ascending.

[mem]

I was just working on some initial tests for LoadProject. I can create a PR for that if that's ok, and expand cases from there.

[sdboyer]

@mem ah, thanks, that'd be great! though it looks like @tro3 is already working on some for LoadProject. Maybe have a look at their PR, see if you can suggest some of those additional cases? Or if not, I'm sure we can find something else :)

[mem]

Sure, no problem. It was more of an accident. I looked at the open issues, spotted this one and started working on that function just to get a feeling of the code structure, etc. I'll look at the coverage for other functions and post here before writing anything else :-)

[tro3]

No worries, @mem - I just looked at #199 and they could be pulled and not step on each other. I'll leave it up to @sdboyer. My PR tonight or tomorrow will also include the ascending bug fix, which I moved to #198, and a (slightly) improved docstring.

[sdboyer]

I think we're doing much better with this now, especially with the harness. I'm gonna close this out in favor of more specific issues 😄