Merge pull request #6810 from rhcarvalho/sanitize-s2i-builder-tests

Merged by openshift-bot

pkg/build/builder/dockerutil_test.go

@@ -10,6 +10,11 @@ type FakeDocker struct {
  pushImageFunc func(opts docker.PushImageOptions, auth docker.AuthConfiguration) error
  buildImageFunc func(opts docker.BuildImageOptions) error
  removeImageFunc func(name string) error
+
+ buildImageCalled bool
+ pushImageCalled bool
+ removeImageCalled bool
+ errPushImage error
 }
 
 func (d *FakeDocker) BuildImage(opts docker.BuildImageOptions) error {
@@ -18,25 +23,22 @@ func (d *FakeDocker) BuildImage(opts docker.BuildImageOptions) error {
  }
  return nil
 }
-
 func (d *FakeDocker) PushImage(opts docker.PushImageOptions, auth docker.AuthConfiguration) error {
+ d.pushImageCalled = true
  if d.pushImageFunc != nil {
  return d.pushImageFunc(opts, auth)
  }
- return nil
+ return d.errPushImage
 }
-
 func (d *FakeDocker) RemoveImage(name string) error {
  if d.removeImageFunc != nil {
  return d.removeImageFunc(name)
  }
  return nil
 }
-
 func (d *FakeDocker) CreateContainer(opts docker.CreateContainerOptions) (*docker.Container, error) {
- return nil, nil
+ return &docker.Container{}, nil
 }
-
 func (d *FakeDocker) DownloadFromContainer(id string, opts docker.DownloadFromContainerOptions) error {
  return nil
 }
@@ -47,7 +49,7 @@ func (d *FakeDocker) RemoveContainer(opts docker.RemoveContainerOptions) error {
  return nil
 }
 func (d *FakeDocker) InspectImage(name string) (*docker.Image, error) {
- return nil, nil
+ return &docker.Image{}, nil
 }
 func TestDockerPush(t *testing.T) {
  verifyFunc := func(opts docker.PushImageOptions, auth docker.AuthConfiguration) error {

pkg/build/builder/sti_test.go

@@ -5,107 +5,66 @@ import (
  "strings"
  "testing"
 
- docker "github.com/fsouza/go-dockerclient"
  kapi "k8s.io/kubernetes/pkg/api"
 
  "github.com/openshift/origin/pkg/build/api"
  "github.com/openshift/origin/pkg/client/testclient"
  "github.com/openshift/origin/pkg/generate/git"
  s2iapi "github.com/openshift/source-to-image/pkg/api"
- "github.com/openshift/source-to-image/pkg/api/validation"
  s2ibuild "github.com/openshift/source-to-image/pkg/build"
 )
 
-type testDockerClient struct {
- buildImageCalled bool
- pushImageCalled bool
- removeImageCalled bool
- errPushImage error
-}
-
-func (client testDockerClient) BuildImage(opts docker.BuildImageOptions) error {
- return nil
-}
-
-func (client testDockerClient) PushImage(opts docker.PushImageOptions, auth docker.AuthConfiguration) error {
- client.pushImageCalled = true
- return client.errPushImage
-}
-
-func (client testDockerClient) RemoveImage(name string) error {
- return nil
-}
-
-func (client testDockerClient) CreateContainer(opts docker.CreateContainerOptions) (*docker.Container, error) {
- return nil, nil
-}
-
-func (client testDockerClient) DownloadFromContainer(id string, opts docker.DownloadFromContainerOptions) error {
- return nil
-}
-
-func (client testDockerClient) PullImage(opts docker.PullImageOptions, auth docker.AuthConfiguration) error {
- return nil
-}
-
-func (client testDockerClient) RemoveContainer(opts docker.RemoveContainerOptions) error {
- return nil
-}
-func (client testDockerClient) InspectImage(name string) (*docker.Image, error) {
- return nil, nil
-}
-
+// testStiBuilderFactory is a mock implementation of builderFactory.
 type testStiBuilderFactory struct {
  getStrategyErr error
  buildError error
 }
 
-type testStiConfigValidator struct {
- errors []validation.ValidationError
-}
-
-// Mock S2I builder factory implementation. Just returns mock S2I builder instances ot error (if set)
+// Builder implements builderFactory. It returns a mock S2IBuilder instance that
+// returns specific errors.
 func (factory testStiBuilderFactory) Builder(config *s2iapi.Config, overrides s2ibuild.Overrides) (s2ibuild.Builder, error) {
- // if there is error set, return this error
+ // Return a strategy error if non-nil.
  if factory.getStrategyErr != nil {
  return nil, factory.getStrategyErr
  }
  return testBuilder{buildError: factory.buildError}, nil
 }
 
+// testBuilder is a mock implementation of s2iapi.Builder.
 type testBuilder struct {
  buildError error
 }
 
-// Build is a mock implementation for STI builder, returns nil result and error if any
+// Build implements s2iapi.Builder. It always returns a mocked build error.
 func (builder testBuilder) Build(config *s2iapi.Config) (*s2iapi.Result, error) {
  return nil, builder.buildError
 }
 
-// creates mock implemenation of STI builder, instrumenting different parts of a process to return errors
-func makeStiBuilder(
- errPushImage error,
- getStrategyErr error,
- buildError error,
- validationErrors []validation.ValidationError) S2IBuilder {
- return *newS2IBuilder(
- testDockerClient{
- errPushImage: errPushImage,
+type testS2IBuilderConfig struct {
+ errPushImage error
+ getStrategyErr error
+ buildError error
+}
+
+// newTestS2IBuilder creates a mock implementation of S2IBuilder, instrumenting
+// different parts to return specific errors according to config.
+func newTestS2IBuilder(config testS2IBuilderConfig) *S2IBuilder {
+ return newS2IBuilder(
+ &FakeDocker{
+ errPushImage: config.errPushImage,
  },
  "/docker.socket",
  testclient.NewSimpleFake().Builds(""),
  makeBuild(),
  git.NewRepository(),
- testStiBuilderFactory{getStrategyErr: getStrategyErr, buildError: buildError},
- testStiConfigValidator{errors: validationErrors},
+ testStiBuilderFactory{
+ getStrategyErr: config.getStrategyErr,
+ buildError: config.buildError,
+ },
+ runtimeConfigValidator{},
  )
 }
 
-// ValidateConfig is a mock implementation for config validator. returns error if set or nil
-func (validator testStiConfigValidator) ValidateConfig(config *s2iapi.Config) []validation.ValidationError {
- return validator.errors
-}
-
 func makeBuild() *api.Build {
  return &api.Build{
  Spec: api.BuildSpec{
@@ -136,40 +95,30 @@ func makeBuild() *api.Build {
 
 func TestDockerBuildError(t *testing.T) {
  expErr := errors.New("Artificial exception: Error building")
- s2iBuilder := makeStiBuilder(expErr, nil, nil, make([]validation.ValidationError, 0))
- err := s2iBuilder.Build()
- if err == nil {
- t.Error("Artificial error expected from build process")
- } else {
- if !strings.Contains(err.Error(), expErr.Error()) {
- t.Errorf("Artificial error expected from build process: \n Returned error: %s\n Expected error: %s", err.Error(), expErr.Error())
- }
+ s2iBuilder := newTestS2IBuilder(testS2IBuilderConfig{
+ buildError: expErr,
+ })
+ if err := s2iBuilder.Build(); err != expErr {
+ t.Errorf("s2iBuilder.Build() = %v; want %v", err, expErr)
  }
 }
 
 func TestPushError(t *testing.T) {
  expErr := errors.New("Artificial exception: Error pushing image")
- s2iBuilder := makeStiBuilder(nil, nil, expErr, make([]validation.ValidationError, 0))
- err := s2iBuilder.Build()
- if err == nil {
- t.Error("Artificial error expected from build process")
- } else {
- if !strings.Contains(err.Error(), expErr.Error()) {
- t.Errorf("Artificial error expected from build process: \n Returned error: %s\n Expected error: %s", err.Error(), expErr.Error())
- }
+ s2iBuilder := newTestS2IBuilder(testS2IBuilderConfig{
+ errPushImage: expErr,
+ })
+ if err := s2iBuilder.Build(); !strings.HasSuffix(err.Error(), expErr.Error()) {
+ t.Errorf("s2iBuilder.Build() = %v; want %v", err, expErr)
  }
 }
 
-// Test error creating s2i builder
 func TestGetStrategyError(t *testing.T) {
  expErr := errors.New("Artificial exception: config error")
- s2iBuilder := makeStiBuilder(nil, expErr, nil, make([]validation.ValidationError, 0))
- err := s2iBuilder.Build()
- if err == nil {
- t.Error("Artificial error expected from build process")
- } else {
- if !strings.Contains(err.Error(), expErr.Error()) {
- t.Errorf("Artificial error expected from build process: \n Returned error: %s\n Expected error: %s", err.Error(), expErr.Error())
- }
+ s2iBuilder := newTestS2IBuilder(testS2IBuilderConfig{
+ getStrategyErr: expErr,
+ })
+ if err := s2iBuilder.Build(); err != expErr {
+ t.Errorf("s2iBuilder.Build() = %v; want %v", err, expErr)
  }
 }