From 774bfea341cac675ccf3075d5b86007e69fb3163 Mon Sep 17 00:00:00 2001
From: Guillaume Tardif <guillaume.tardif@docker.com>
Date: Tue, 2 Jun 2020 23:33:41 +0200
Subject: [PATCH] Refactoring, add unit test or various interactive context
 creation

---
 azure/aci.go                     |  45 -------
 azure/aciResourceGroupHelper.go  | 105 ++++++++++++++++
 azure/backend.go                 |   3 +-
 azure/context.go                 |  52 ++++----
 azure/context_test.go            | 210 +++++++++++++++++++++++++++++++
 tests/aci-e2e/e2e-aci_test.go    |  23 ++--
 tests/aci-e2e/storage/storage.go |   3 +-
 7 files changed, 362 insertions(+), 79 deletions(-)
 create mode 100644 azure/aciResourceGroupHelper.go
 create mode 100644 azure/context_test.go

diff --git a/azure/aci.go b/azure/aci.go
index 20aef8412..ebce23044 100644
--- a/azure/aci.go
+++ b/azure/aci.go
@@ -9,8 +9,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/Azure/azure-sdk-for-go/profiles/2019-03-01/resources/mgmt/resources"
-	"github.com/Azure/azure-sdk-for-go/profiles/preview/preview/subscription/mgmt/subscription"
 	"github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-10-01/containerinstance"
 	"github.com/Azure/go-autorest/autorest"
 	"github.com/Azure/go-autorest/autorest/to"
@@ -20,8 +18,6 @@ import (
 	"github.com/pkg/errors"
 
 	"github.com/docker/api/azure/login"
-	"github.com/docker/api/errdefs"
-
 	"github.com/docker/api/context/store"
 )
 
@@ -257,44 +253,3 @@ func getContainerClient(subscriptionID string) (containerinstance.ContainerClien
 	containerClient.Authorizer = auth
 	return containerClient, nil
 }
-
-func getSubscriptionsClient() (subscription.SubscriptionsClient, error) {
-	subc := subscription.NewSubscriptionsClient()
-	authorizer, err := login.NewAuthorizerFromLogin()
-	if err != nil {
-		return subscription.SubscriptionsClient{}, errors.Wrap(errdefs.ErrLoginFailed, err.Error())
-	}
-	subc.Authorizer = authorizer
-	return subc, nil
-}
-
-func getGroupsClient(subscriptionID string) resources.GroupsClient {
-	groupsClient := resources.NewGroupsClient(subscriptionID)
-	authorizer, _ := login.NewAuthorizerFromLogin()
-	groupsClient.Authorizer = authorizer
-	return groupsClient
-}
-
-func getSubscriptionIDs(ctx context.Context) ([]subscription.Model, error) {
-	c, err := getSubscriptionsClient()
-	if err != nil {
-		return nil, err
-	}
-	res, err := c.List(ctx)
-	if err != nil {
-		return nil, err
-	}
-	subs := res.Values()
-
-	if len(subs) == 0 {
-		return nil, errors.New("no subscriptions found")
-	}
-	for res.NotDone() {
-		err = res.NextWithContext(ctx)
-		if err != nil {
-			return nil, err
-		}
-		subs = append(subs, res.Values()...)
-	}
-	return subs, nil
-}
diff --git a/azure/aciResourceGroupHelper.go b/azure/aciResourceGroupHelper.go
new file mode 100644
index 000000000..d63770706
--- /dev/null
+++ b/azure/aciResourceGroupHelper.go
@@ -0,0 +1,105 @@
+package azure
+
+import (
+	"context"
+
+	"github.com/Azure/azure-sdk-for-go/profiles/2019-03-01/resources/mgmt/resources"
+	"github.com/Azure/azure-sdk-for-go/profiles/preview/preview/subscription/mgmt/subscription"
+	"github.com/pkg/errors"
+
+	"github.com/docker/api/azure/login"
+	"github.com/docker/api/errdefs"
+)
+
+// ACIResourceGroupHelper interface to manage resource groups and subscription IDs
+type ACIResourceGroupHelper interface {
+	GetSubscriptionIDs(ctx context.Context) ([]subscription.Model, error)
+	ListGroups(ctx context.Context, subscriptionID string) ([]resources.Group, error)
+	GetGroup(ctx context.Context, subscriptionID string, groupName string) (resources.Group, error)
+	CreateOrUpdate(ctx context.Context, subscriptionID string, resourceGroupName string, parameters resources.Group) (result resources.Group, err error)
+	Delete(ctx context.Context, subscriptionID string, resourceGroupName string) error
+}
+
+type aciResourceGroupHelperImpl struct {
+}
+
+// NewACIResourceGroupHelper create a new ACIResourceGroupHelper
+func NewACIResourceGroupHelper() ACIResourceGroupHelper {
+	return aciResourceGroupHelperImpl{}
+}
+
+// GetGroup get a resource group from its name
+func (mgt aciResourceGroupHelperImpl) GetGroup(ctx context.Context, subscriptionID string, groupName string) (resources.Group, error) {
+	gc := getGroupsClient(subscriptionID)
+	return gc.Get(ctx, groupName)
+}
+
+// ListGroups list resource groups
+func (mgt aciResourceGroupHelperImpl) ListGroups(ctx context.Context, subscriptionID string) ([]resources.Group, error) {
+	gc := getGroupsClient(subscriptionID)
+	groupResponse, err := gc.List(ctx, "", nil)
+	if err != nil {
+		return nil, err
+	}
+
+	groups := groupResponse.Values()
+	return groups, nil
+}
+
+// CreateOrUpdate create or update a resource group
+func (mgt aciResourceGroupHelperImpl) CreateOrUpdate(ctx context.Context, subscriptionID string, resourceGroupName string, parameters resources.Group) (result resources.Group, err error) {
+	gc := getGroupsClient(subscriptionID)
+	return gc.CreateOrUpdate(ctx, resourceGroupName, parameters)
+}
+
+// Delete deletes a resource group
+func (mgt aciResourceGroupHelperImpl) Delete(ctx context.Context, subscriptionID string, resourceGroupName string) (err error) {
+	gc := getGroupsClient(subscriptionID)
+	future, err := gc.Delete(ctx, resourceGroupName)
+	if err != nil {
+		return err
+	}
+	return future.WaitForCompletionRef(ctx, gc.Client)
+}
+
+// GetSubscriptionIDs Return available subscription IDs based on azure login
+func (mgt aciResourceGroupHelperImpl) GetSubscriptionIDs(ctx context.Context) ([]subscription.Model, error) {
+	c, err := getSubscriptionsClient()
+	if err != nil {
+		return nil, err
+	}
+	res, err := c.List(ctx)
+	if err != nil {
+		return nil, err
+	}
+	subs := res.Values()
+
+	if len(subs) == 0 {
+		return nil, errors.New("no subscriptions found")
+	}
+	for res.NotDone() {
+		err = res.NextWithContext(ctx)
+		if err != nil {
+			return nil, err
+		}
+		subs = append(subs, res.Values()...)
+	}
+	return subs, nil
+}
+
+func getSubscriptionsClient() (subscription.SubscriptionsClient, error) {
+	subc := subscription.NewSubscriptionsClient()
+	authorizer, err := login.NewAuthorizerFromLogin()
+	if err != nil {
+		return subscription.SubscriptionsClient{}, errors.Wrap(errdefs.ErrLoginFailed, err.Error())
+	}
+	subc.Authorizer = authorizer
+	return subc, nil
+}
+
+func getGroupsClient(subscriptionID string) resources.GroupsClient {
+	groupsClient := resources.NewGroupsClient(subscriptionID)
+	authorizer, _ := login.NewAuthorizerFromLogin()
+	groupsClient.Authorizer = authorizer
+	return groupsClient
+}
diff --git a/azure/backend.go b/azure/backend.go
index 073120e19..b990424dc 100644
--- a/azure/backend.go
+++ b/azure/backend.go
@@ -284,5 +284,6 @@ func (cs *aciCloudService) Login(ctx context.Context, params map[string]string)
 }
 
 func (cs *aciCloudService) CreateContextData(ctx context.Context, params map[string]string) (interface{}, string, error) {
-	return createContextData(ctx, params, cliUserSelector{})
+	contextHelper := newContextCreateHelper()
+	return contextHelper.createContextData(ctx, params)
 }
diff --git a/azure/context.go b/azure/context.go
index 640b77b22..9c8d03ac1 100644
--- a/azure/context.go
+++ b/azure/context.go
@@ -30,53 +30,59 @@ package azure
 import (
 	"context"
 	"fmt"
-
-	"github.com/Azure/azure-sdk-for-go/profiles/preview/preview/subscription/mgmt/subscription"
-	"github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources"
-
 	"os"
 
 	"github.com/AlecAivazis/survey/v2"
-
-	"github.com/docker/api/context/store"
-
+	"github.com/Azure/azure-sdk-for-go/profiles/preview/preview/subscription/mgmt/subscription"
+	"github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources"
 	"github.com/google/uuid"
 	"github.com/pkg/errors"
 	"github.com/tj/survey/terminal"
+
+	"github.com/docker/api/context/store"
 )
 
-func createContextData(ctx context.Context, opts map[string]string, selector userSelector) (interface{}, string, error) {
+type contextCreateACIHelper struct {
+	selector            userSelector
+	resourceGroupHelper ACIResourceGroupHelper
+}
+
+func newContextCreateHelper() contextCreateACIHelper {
+	return contextCreateACIHelper{
+		selector:            cliUserSelector{},
+		resourceGroupHelper: aciResourceGroupHelperImpl{},
+	}
+}
+
+func (helper contextCreateACIHelper) createContextData(ctx context.Context, opts map[string]string) (interface{}, string, error) {
 	var subscriptionID string
 	if opts["aciSubscriptionID"] != "" {
 		subscriptionID = opts["aciSubscriptionID"]
 	} else {
-		subs, err := getSubscriptionIDs(ctx)
+		subs, err := helper.resourceGroupHelper.GetSubscriptionIDs(ctx)
 		if err != nil {
 			return nil, "", err
 		}
-		subscriptionID, err = chooseSub(subs, selector)
+		subscriptionID, err = helper.chooseSub(subs)
 		if err != nil {
 			return nil, "", err
 		}
 	}
 
-	gc := getGroupsClient(subscriptionID)
 	var group resources.Group
 	var err error
 
 	if opts["aciResourceGroup"] != "" {
-		group, err = gc.Get(ctx, opts["aciResourceGroup"])
+		group, err = helper.resourceGroupHelper.GetGroup(ctx, subscriptionID, opts["aciResourceGroup"])
 		if err != nil {
 			return nil, "", errors.Wrapf(err, "Could not find resource group %q", opts["aciResourceGroup"])
 		}
 	} else {
-		groupResponse, err := gc.List(ctx, "", nil)
+		groups, err := helper.resourceGroupHelper.ListGroups(ctx, subscriptionID)
 		if err != nil {
 			return nil, "", err
 		}
-
-		groups := groupResponse.Values()
-		group, err = chooseGroup(ctx, gc, opts, groups, selector)
+		group, err = helper.chooseGroup(ctx, subscriptionID, opts, groups)
 		if err != nil {
 			return nil, "", err
 		}
@@ -99,12 +105,12 @@ func createContextData(ctx context.Context, opts map[string]string, selector use
 	}, description, nil
 }
 
-func createGroup(ctx context.Context, gc resources.GroupsClient, location string) (resources.Group, error) {
+func (helper contextCreateACIHelper) createGroup(ctx context.Context, subscriptionID, location string) (resources.Group, error) {
 	if location == "" {
 		location = "eastus"
 	}
 	gid := uuid.New().String()
-	g, err := gc.CreateOrUpdate(ctx, gid, resources.Group{
+	g, err := helper.resourceGroupHelper.CreateOrUpdate(ctx, subscriptionID, gid, resources.Group{
 		Location: &location,
 	})
 	if err != nil {
@@ -116,13 +122,13 @@ func createGroup(ctx context.Context, gc resources.GroupsClient, location string
 	return g, nil
 }
 
-func chooseGroup(ctx context.Context, gc resources.GroupsClient, opts map[string]string, groups []resources.Group, selector userSelector) (resources.Group, error) {
+func (helper contextCreateACIHelper) chooseGroup(ctx context.Context, subscriptionID string, opts map[string]string, groups []resources.Group) (resources.Group, error) {
 	groupNames := []string{"create a new resource group"}
 	for _, g := range groups {
 		groupNames = append(groupNames, fmt.Sprintf("%s (%s)", *g.Name, *g.Location))
 	}
 
-	group, err := selector.userSelect("Choose a resource group", groupNames)
+	group, err := helper.selector.userSelect("Select a resource group", groupNames)
 	if err != nil {
 		if err == terminal.InterruptErr {
 			os.Exit(0)
@@ -132,13 +138,13 @@ func chooseGroup(ctx context.Context, gc resources.GroupsClient, opts map[string
 	}
 
 	if group == 0 {
-		return createGroup(ctx, gc, opts["aciLocation"])
+		return helper.createGroup(ctx, subscriptionID, opts["aciLocation"])
 	}
 
 	return groups[group-1], nil
 }
 
-func chooseSub(subs []subscription.Model, selector userSelector) (string, error) {
+func (helper contextCreateACIHelper) chooseSub(subs []subscription.Model) (string, error) {
 	if len(subs) == 1 {
 		sub := subs[0]
 		fmt.Println("Using only available subscription : " + *sub.DisplayName + "(" + *sub.SubscriptionID + ")")
@@ -148,7 +154,7 @@ func chooseSub(subs []subscription.Model, selector userSelector) (string, error)
 	for _, sub := range subs {
 		options = append(options, *sub.DisplayName+"("+*sub.SubscriptionID+")")
 	}
-	selected, err := selector.userSelect("Select a subscription ID", options)
+	selected, err := helper.selector.userSelect("Select a subscription ID", options)
 	if err != nil {
 		if err == terminal.InterruptErr {
 			os.Exit(0)
diff --git a/azure/context_test.go b/azure/context_test.go
new file mode 100644
index 000000000..a927e6157
--- /dev/null
+++ b/azure/context_test.go
@@ -0,0 +1,210 @@
+package azure
+
+import (
+	"context"
+	"testing"
+
+	"github.com/Azure/azure-sdk-for-go/profiles/2019-03-01/resources/mgmt/resources"
+	"github.com/Azure/azure-sdk-for-go/profiles/preview/preview/subscription/mgmt/subscription"
+	"github.com/Azure/go-autorest/autorest/to"
+	"github.com/pkg/errors"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/suite"
+
+	"github.com/docker/api/context/store"
+
+	. "github.com/onsi/gomega"
+)
+
+type ContextSuiteTest struct {
+	suite.Suite
+	mockUserSelector       *MockUserSelector
+	mockResourceGroupHeper *MockResourceGroupHelper
+	contextCreateHelper    contextCreateACIHelper
+}
+
+func (suite *ContextSuiteTest) BeforeTest(suiteName, testName string) {
+	suite.mockUserSelector = &MockUserSelector{}
+	suite.mockResourceGroupHeper = &MockResourceGroupHelper{}
+	suite.contextCreateHelper = contextCreateACIHelper{
+		suite.mockUserSelector,
+		suite.mockResourceGroupHeper,
+	}
+}
+
+func (suite *ContextSuiteTest) TestCreateSpecifiedSubscriptionAndGroup() {
+	ctx := context.TODO()
+	opts := options("1234", "myResourceGroup")
+	suite.mockResourceGroupHeper.On("GetGroup", ctx, "1234", "myResourceGroup").Return(group("myResourceGroup", "eastus"), nil)
+
+	data, description, err := suite.contextCreateHelper.createContextData(ctx, opts)
+
+	Expect(err).To(BeNil())
+	Expect(description).To(Equal("myResourceGroup@eastus"))
+	Expect(data).To(Equal(aciContext("1234", "myResourceGroup", "eastus")))
+}
+
+func (suite *ContextSuiteTest) TestErrorOnNonExistentResourceGroup() {
+	ctx := context.TODO()
+	opts := options("1234", "myResourceGroup")
+	notFoundError := errors.New(`Not Found: "myResourceGroup"`)
+	suite.mockResourceGroupHeper.On("GetGroup", ctx, "1234", "myResourceGroup").Return(resources.Group{}, notFoundError)
+
+	data, description, err := suite.contextCreateHelper.createContextData(ctx, opts)
+
+	Expect(data).To(BeNil())
+	Expect(description).To(Equal(""))
+	Expect(err.Error()).To(Equal("Could not find resource group \"myResourceGroup\": Not Found: \"myResourceGroup\""))
+}
+
+func (suite *ContextSuiteTest) TestCreateNewResourceGroup() {
+	ctx := context.TODO()
+	opts := options("1234", "")
+	suite.mockResourceGroupHeper.On("GetGroup", ctx, "1234", "myResourceGroup").Return(group("myResourceGroup", "eastus"), nil)
+
+	selectOptions := []string{"create a new resource group", "group1 (eastus)", "group2 (westeurope)"}
+	suite.mockUserSelector.On("userSelect", "Select a resource group", selectOptions).Return(0, nil)
+	suite.mockResourceGroupHeper.On("CreateOrUpdate", ctx, "1234", mock.AnythingOfType("string"), mock.AnythingOfType("resources.Group")).Return(group("newResourceGroup", "eastus"), nil)
+	suite.mockResourceGroupHeper.On("ListGroups", ctx, "1234").Return([]resources.Group{
+		group("group1", "eastus"),
+		group("group2", "westeurope"),
+	}, nil)
+
+	data, description, err := suite.contextCreateHelper.createContextData(ctx, opts)
+
+	Expect(err).To(BeNil())
+	Expect(description).To(Equal("newResourceGroup@eastus"))
+	Expect(data).To(Equal(aciContext("1234", "newResourceGroup", "eastus")))
+}
+
+func (suite *ContextSuiteTest) TestSelectExistingResourceGroup() {
+	ctx := context.TODO()
+	opts := options("1234", "")
+	selectOptions := []string{"create a new resource group", "group1 (eastus)", "group2 (westeurope)"}
+	suite.mockUserSelector.On("userSelect", "Select a resource group", selectOptions).Return(2, nil)
+	suite.mockResourceGroupHeper.On("ListGroups", ctx, "1234").Return([]resources.Group{
+		group("group1", "eastus"),
+		group("group2", "westeurope"),
+	}, nil)
+
+	data, description, err := suite.contextCreateHelper.createContextData(ctx, opts)
+
+	Expect(err).To(BeNil())
+	Expect(description).To(Equal("group2@westeurope"))
+	Expect(data).To(Equal(aciContext("1234", "group2", "westeurope")))
+}
+
+func (suite *ContextSuiteTest) TestSelectSingleSubscriptionIdAndExistingResourceGroup() {
+	ctx := context.TODO()
+	opts := options("", "")
+	suite.mockResourceGroupHeper.On("GetSubscriptionIDs", ctx).Return([]subscription.Model{subModel("123456", "Subscription1")}, nil)
+
+	selectOptions := []string{"create a new resource group", "group1 (eastus)", "group2 (westeurope)"}
+	suite.mockUserSelector.On("userSelect", "Select a resource group", selectOptions).Return(2, nil)
+	suite.mockResourceGroupHeper.On("ListGroups", ctx, "123456").Return([]resources.Group{
+		group("group1", "eastus"),
+		group("group2", "westeurope"),
+	}, nil)
+
+	data, description, err := suite.contextCreateHelper.createContextData(ctx, opts)
+
+	Expect(err).To(BeNil())
+	Expect(description).To(Equal("group2@westeurope"))
+	Expect(data).To(Equal(aciContext("123456", "group2", "westeurope")))
+}
+
+func (suite *ContextSuiteTest) TestSelectSubscriptionIdAndExistingResourceGroup() {
+	ctx := context.TODO()
+	opts := options("", "")
+	sub1 := subModel("1234", "Subscription1")
+	sub2 := subModel("5678", "Subscription2")
+
+	suite.mockResourceGroupHeper.On("GetSubscriptionIDs", ctx).Return([]subscription.Model{sub1, sub2}, nil)
+
+	selectOptions := []string{"Subscription1(1234)", "Subscription2(5678)"}
+	suite.mockUserSelector.On("userSelect", "Select a subscription ID", selectOptions).Return(1, nil)
+	selectOptions = []string{"create a new resource group", "group1 (eastus)", "group2 (westeurope)"}
+	suite.mockUserSelector.On("userSelect", "Select a resource group", selectOptions).Return(2, nil)
+	suite.mockResourceGroupHeper.On("ListGroups", ctx, "5678").Return([]resources.Group{
+		group("group1", "eastus"),
+		group("group2", "westeurope"),
+	}, nil)
+
+	data, description, err := suite.contextCreateHelper.createContextData(ctx, opts)
+
+	Expect(err).To(BeNil())
+	Expect(description).To(Equal("group2@westeurope"))
+	Expect(data).To(Equal(aciContext("5678", "group2", "westeurope")))
+}
+
+func subModel(subID string, display string) subscription.Model {
+	return subscription.Model{
+		SubscriptionID: to.StringPtr(subID),
+		DisplayName:    to.StringPtr(display),
+	}
+}
+
+func group(groupName string, location string) resources.Group {
+	return resources.Group{
+		Name:     to.StringPtr(groupName),
+		Location: to.StringPtr(location),
+	}
+}
+
+func aciContext(subscriptionID string, resourceGroupName string, location string) store.AciContext {
+	return store.AciContext{
+		SubscriptionID: subscriptionID,
+		Location:       location,
+		ResourceGroup:  resourceGroupName,
+	}
+}
+
+func options(subscriptionID string, resourceGroupName string) map[string]string {
+	return map[string]string{
+		"aciSubscriptionID": subscriptionID,
+		"aciResourceGroup":  resourceGroupName,
+	}
+}
+
+func TestContextSuite(t *testing.T) {
+	RegisterTestingT(t)
+	suite.Run(t, new(ContextSuiteTest))
+}
+
+type MockUserSelector struct {
+	mock.Mock
+}
+
+func (s *MockUserSelector) userSelect(message string, options []string) (int, error) {
+	args := s.Called(message, options)
+	return args.Int(0), args.Error(1)
+}
+
+type MockResourceGroupHelper struct {
+	mock.Mock
+}
+
+func (s *MockResourceGroupHelper) GetSubscriptionIDs(ctx context.Context) ([]subscription.Model, error) {
+	args := s.Called(ctx)
+	return args.Get(0).([]subscription.Model), args.Error(1)
+}
+
+func (s *MockResourceGroupHelper) ListGroups(ctx context.Context, subscriptionID string) ([]resources.Group, error) {
+	args := s.Called(ctx, subscriptionID)
+	return args.Get(0).([]resources.Group), args.Error(1)
+}
+
+func (s *MockResourceGroupHelper) GetGroup(ctx context.Context, subscriptionID string, groupName string) (resources.Group, error) {
+	args := s.Called(ctx, subscriptionID, groupName)
+	return args.Get(0).(resources.Group), args.Error(1)
+}
+
+func (s *MockResourceGroupHelper) CreateOrUpdate(ctx context.Context, subscriptionID string, resourceGroupName string, parameters resources.Group) (result resources.Group, err error) {
+	args := s.Called(ctx, subscriptionID, resourceGroupName, parameters)
+	return args.Get(0).(resources.Group), args.Error(1)
+}
+
+func (s *MockResourceGroupHelper) Delete(ctx context.Context, subscriptionID string, resourceGroupName string) (err error) {
+	args := s.Called(ctx, subscriptionID, resourceGroupName)
+	return args.Error(0)
+}
diff --git a/tests/aci-e2e/e2e-aci_test.go b/tests/aci-e2e/e2e-aci_test.go
index c0678e28e..69141a1b3 100644
--- a/tests/aci-e2e/e2e-aci_test.go
+++ b/tests/aci-e2e/e2e-aci_test.go
@@ -7,6 +7,8 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/docker/api/azure"
+
 	"github.com/Azure/azure-sdk-for-go/profiles/2019-03-01/resources/mgmt/resources"
 	azure_storage "github.com/Azure/azure-sdk-for-go/profiles/2019-03-01/storage/mgmt/storage"
 	"github.com/Azure/azure-storage-file-go/azfile"
@@ -63,7 +65,8 @@ func (s *E2eACISuite) TestContextDefault() {
 func (s *E2eACISuite) TestACIBackend() {
 	It("creates a new aci context for tests", func() {
 		setupTestResourceGroup(resourceGroupName)
-		models, err := azure.getSubscriptionIDs(context.TODO())
+		helper := azure.NewACIResourceGroupHelper()
+		models, err := helper.GetSubscriptionIDs(context.TODO())
 		Expect(err).To(BeNil())
 		subscriptionID = *models[0].SubscriptionID
 
@@ -172,13 +175,14 @@ func (s *E2eACISuite) TestACIBackend() {
 }
 
 const (
-	testStorageAccountName = "dockertestaccountname"
-	testShareName          = "dockertestsharename"
+	testStorageAccountName = "dockertestaccount"
+	testShareName          = "dockertestshare"
 	testFileContent        = "Volume mounted with success!"
 	testFileName           = "index.html"
 )
 
 func createStorageAccount(aciContext store.AciContext, accountName string) azure_storage.Account {
+	log.Println("Creating storage account " + accountName)
 	storageAccount, err := storage.CreateStorageAccount(context.TODO(), aciContext, accountName)
 	Expect(err).To(BeNil())
 	Expect(*storageAccount.Name).To(Equal(accountName))
@@ -195,6 +199,7 @@ func getStorageKeys(aciContext store.AciContext, storageAccountName string) []az
 }
 
 func deleteStorageAccount(aciContext store.AciContext) {
+	log.Println("Deleting storage account " + testStorageAccountName)
 	_, err := storage.DeleteStorageAccount(context.TODO(), aciContext, testStorageAccountName)
 	Expect(err).To(BeNil())
 }
@@ -227,10 +232,10 @@ func TestE2eACI(t *testing.T) {
 func setupTestResourceGroup(groupName string) {
 	log.Println("Creating resource group " + resourceGroupName)
 	ctx := context.TODO()
-	models, err := azure.getSubscriptionIDs(ctx)
+	helper := azure.NewACIResourceGroupHelper()
+	models, err := helper.GetSubscriptionIDs(ctx)
 	Expect(err).To(BeNil())
-	gc := azure.getGroupsClient(*models[0].SubscriptionID)
-	_, err = gc.CreateOrUpdate(ctx, groupName, resources.Group{
+	_, err = helper.CreateOrUpdate(ctx, *models[0].SubscriptionID, groupName, resources.Group{
 		Location: to.StringPtr(location),
 	})
 	Expect(err).To(BeNil())
@@ -239,9 +244,9 @@ func setupTestResourceGroup(groupName string) {
 func deleteResourceGroup(groupName string) {
 	log.Println("Deleting resource group " + resourceGroupName)
 	ctx := context.TODO()
-	models, err := azure.getSubscriptionIDs(ctx)
+	helper := azure.NewACIResourceGroupHelper()
+	models, err := helper.GetSubscriptionIDs(ctx)
 	Expect(err).To(BeNil())
-	gc := azure.getGroupsClient(*models[0].SubscriptionID)
-	_, err = gc.Delete(ctx, groupName)
+	err = helper.Delete(ctx, *models[0].SubscriptionID, groupName)
 	Expect(err).To(BeNil())
 }
diff --git a/tests/aci-e2e/storage/storage.go b/tests/aci-e2e/storage/storage.go
index 8d616db95..d729d8501 100644
--- a/tests/aci-e2e/storage/storage.go
+++ b/tests/aci-e2e/storage/storage.go
@@ -2,6 +2,7 @@ package storage
 
 import (
 	"context"
+	"errors"
 
 	"github.com/Azure/azure-sdk-for-go/profiles/2019-03-01/storage/mgmt/storage"
 	"github.com/Azure/go-autorest/autorest"
@@ -25,7 +26,7 @@ func CreateStorageAccount(ctx context.Context, aciContext store.AciContext, acco
 		return storage.Account{}, err
 	}
 	if !*result.NameAvailable {
-		return storage.Account{}, err
+		return storage.Account{}, errors.New("storage account name already exists" + accountName)
 	}
 
 	future, err := storageAccountsClient.Create(