Injectable builder (and friends) #125

merged 6 commits into from Feb 25, 2014

2 participants


The goal of these changes is to make everything easier to test, and follow the flow of control through from the top down. Tests can now e.g. inject a faked out docker client to assert that the builder does the right thing, or a faked out runner to assert that the various hooks execute the correct build tasks.

We've made an attempt to break what's effectively a lot of churn into smaller commits that act as a progression. Also made sure that existing singletons are still initialized only once (i.e. docker.DefaultClient).

This is still a WIP. More to come!

/cc @hiremaga

vito added some commits Feb 24, 2014
@vito vito fix handler/testing test imports
Signed-off-by: Abhijit Hiremagalur <>
@vito vito add constructor for Builder
this makes it easier to track required dependencies as they change

(todo: actually, like, use it for required dependencies)
@vito vito introduce Queue object
this is an intermediate step towards pushing configuration up.

Signed-off-by: Abhijit Hiremagalur <>
@vito vito inject docker client into Builder
Signed-off-by: Abhijit Hiremagalur <>
@vito vito split build construction out of worker
(todo: inject runner into worker)

Signed-off-by: Abhijit Hiremagalur <>
@vito vito provide runner to workers, use it for all builds
Signed-off-by: Abhijit Hiremagalur <>
@bradrydzewski member

looking good! you mentioned wip. do you want me to merge these changes now? or do you want to keep working on the PR and merge when complete?


These commits should be done as-is, so merging them would be excellent; we can continue refactoring in later PRs. I've rebased my privileged-builds work on top of this, and that should be ready to go soon as well. Thanks!

@bradrydzewski bradrydzewski merged commit c0adf45 into drone:master Feb 25, 2014

1 check passed

Details default The build succeeded on
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment