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

(Also) Run test harness in-process #510

Closed
sdboyer opened this Issue May 3, 2017 · 5 comments

Comments

Projects
None yet
3 participants
@sdboyer
Copy link
Member

sdboyer commented May 3, 2017

Right now, the test harness is executed out-of-process by compiling an instance of dep and Exec()ing it. It'd be lovely if we could also make it work in-process, by mocking up the commands as needed and just calling them directly.

Doing this would provide us with proper code coverage information. While I'm not a coverage zealot, I do think it's very valuable for us to at least know which parts of our code are covered by tests, and which aren't.

If there's a way to get coverage information from the subprocess, though, I'm probably fine with that, too.

@sdboyer sdboyer changed the title [Also] Run test harness in-process (Also) Run test harness in-process May 3, 2017

@carolynvs

This comment has been minimized.

Copy link
Collaborator

carolynvs commented May 3, 2017

Another plus side of in-proc tests is easier debugging. When it spawns a separate process, I end up having to put in lots of temporary debug statements. 😀 Or maybe I just need to level up my debugger-fu.

@sdboyer

This comment has been minimized.

Copy link
Member

sdboyer commented May 3, 2017

Sure seems like another benefit to me 😄

/me confesses to not using a step-through debugger (in Go) 😝

@carolynvs

This comment has been minimized.

Copy link
Collaborator

carolynvs commented May 4, 2017

I only use delve in anger. 😇

@jmank88

This comment has been minimized.

Copy link
Collaborator

jmank88 commented May 5, 2017

I can work on this issue.
After exploring the code for a bit, I expect the largest part of the change will be organizing logging, since there are global function calls, global state, and direct os.Std* access sprinkled around which need to be captured by the tests. Some of these already have preexisting TODOs by virtue of being global, so that's another bonus for this issue.

@sdboyer

This comment has been minimized.

Copy link
Member

sdboyer commented May 5, 2017

@jmank88 aaaaawesome!! yes, your survey of the code matches with my sense of what we'd need to do - a bunch more encapsulation to get rid of the global state. That's some tech debt I would LOVE to pay down.

Once that's done, I suspect the rest oughtn't be that hard. Even better, it'll likely force us to maintain a higher level of quality/avoid global state in the future! 🎉

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