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

Replace most subprocess-spawning tests w/ dependency injection #25

Closed
bitprophet opened this Issue Oct 14, 2012 · 1 comment

Comments

Projects
None yet
1 participant
@bitprophet
Member

bitprophet commented Oct 14, 2012

Many tests that are testing specific behavior of e.g run() take the easy route and call e.g. run('true', xxx), then introspect the result. This of course runs an actual subprocess and makes for a slower test suite ("only" ~4s right now, but it will keep growing.) Some even spawn a subprocess that spawns another subprocess! (E.g. the end of #21's new tests do this to get around pty stuff.)

Only the highest level sanity/integration tests really need to do this; the rest should be able to work with a dependency-injected mock object that behaves as needed w/o spawning a real subprocess.

Required:

  • Choose a mocking lib if we aren't already using one.
  • Change run to accept a runner argument & factor out the pexpect/subprocess using chunk of current function body.
  • Update most tests to do run(runner=<mock>).
@bitprophet

This comment has been minimized.

Member

bitprophet commented May 28, 2014

Completely forgot about this ticket - I took care of this before tackling #120 / #123 and the test suite went from ~9s to ~3s. Most of what remains are actual tests of run itself which I am loathe to toss out into the integration suite as they're crucial. Let's call this done for now. If test time grows again we can make a 2nd pass later.

@bitprophet bitprophet closed this May 28, 2014

bitprophet added a commit that referenced this issue May 28, 2014

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