From bfa601cd47b8eb03d1c1641bafee44fb70a26378 Mon Sep 17 00:00:00 2001 From: Daniel Mieg <56156797+DanielMieg@users.noreply.github.com> Date: Fri, 31 Jul 2020 14:43:23 +0200 Subject: [PATCH] Improve testability of abap steps (#1840) * Test * Test * Test abapEnvironmentPullGitRepo step * Move mock functions * Add package for mock * Move mock --- cmd/abapEnvironmentPullGitRepo.go | 48 +++++++++++++++++------ cmd/abapEnvironmentPullGitRepo_test.go | 53 +++++++++++++++++++++++--- cmd/abapEnvironmentRunATCCheck.go | 11 ++++-- cmd/abapEnvironmentRunATCCheck_test.go | 17 +++++++-- pkg/abaputils/abaputils.go | 35 ++++++++++++++++- pkg/abaputils/abaputils_test.go | 32 +++++++++++----- 6 files changed, 160 insertions(+), 36 deletions(-) diff --git a/cmd/abapEnvironmentPullGitRepo.go b/cmd/abapEnvironmentPullGitRepo.go index 6c08d4fe4..9bc1d944a 100644 --- a/cmd/abapEnvironmentPullGitRepo.go +++ b/cmd/abapEnvironmentPullGitRepo.go @@ -20,7 +20,32 @@ import ( "github.com/pkg/errors" ) -func abapEnvironmentPullGitRepo(options abapEnvironmentPullGitRepoOptions, telemetryData *telemetry.CustomData) error { +func abapEnvironmentPullGitRepo(options abapEnvironmentPullGitRepoOptions, telemetryData *telemetry.CustomData) { + + // for command execution use Command + c := command.Command{} + // reroute command output to logging framework + c.Stdout(log.Writer()) + c.Stderr(log.Writer()) + + var autils = abaputils.AbapUtils{ + Exec: &c, + } + + client := piperhttp.Client{} + + // for http calls import piperhttp "github.com/SAP/jenkins-library/pkg/http" + // and use a &piperhttp.Client{} in a custom system + // Example: step checkmarxExecuteScan.go + + // error situations should stop execution through log.Entry().Fatal() call which leads to an os.Exit(1) in the end + err := runAbapEnvironmentPullGitRepo(&options, telemetryData, &autils, &client) + if err != nil { + log.Entry().WithError(err).Fatal("step execution failed") + } +} + +func runAbapEnvironmentPullGitRepo(options *abapEnvironmentPullGitRepoOptions, telemetryData *telemetry.CustomData, com abaputils.Communication, client piperhttp.Sender) error { // Mapping for options subOptions := abaputils.AbapEnvironmentOptions{} @@ -34,19 +59,17 @@ func abapEnvironmentPullGitRepo(options abapEnvironmentPullGitRepoOptions, telem subOptions.Password = options.Password subOptions.Username = options.Username - var c command.ExecRunner = &command.Command{} - // Determine the host, user and password, either via the input parameters or via a cloud foundry service key - connectionDetails, errorGetInfo := abaputils.GetAbapCommunicationArrangementInfo(subOptions, c, "/sap/opu/odata/sap/MANAGE_GIT_REPOSITORY/Pull") + connectionDetails, errorGetInfo := com.GetAbapCommunicationArrangementInfo(subOptions, "/sap/opu/odata/sap/MANAGE_GIT_REPOSITORY/Pull") if errorGetInfo != nil { - log.Entry().WithError(errorGetInfo).Fatal("Parameters for the ABAP Connection not available") + return errors.Wrap(errorGetInfo, "Parameters for the ABAP Connection not available") } // Configuring the HTTP Client and CookieJar - client := piperhttp.Client{} + cookieJar, errorCookieJar := cookiejar.New(nil) if errorCookieJar != nil { - log.Entry().WithError(errorCookieJar).Fatal("Could not create a Cookie Jar") + return errors.Wrap(errorCookieJar, "Could not create a Cookie Jar") } clientOptions := piperhttp.ClientOptions{ MaxRequestDuration: 180 * time.Second, @@ -65,18 +88,19 @@ func abapEnvironmentPullGitRepo(options abapEnvironmentPullGitRepoOptions, telem log.Entry().Info("-------------------------") // Triggering the Pull of the repository into the ABAP Environment system - uriConnectionDetails, errorTriggerPull := triggerPull(repositoryName, connectionDetails, &client) + uriConnectionDetails, errorTriggerPull := triggerPull(repositoryName, connectionDetails, client) if errorTriggerPull != nil { - log.Entry().WithError(errorTriggerPull).Fatal("Pull of " + repositoryName + " failed on the ABAP System") + return errors.Wrapf(errorTriggerPull, "Pull of '%s' failed on the ABAP System", repositoryName) + } // Polling the status of the repository import on the ABAP Environment system - status, errorPollEntity := pollEntity(repositoryName, uriConnectionDetails, &client, pollIntervall) + status, errorPollEntity := pollEntity(repositoryName, uriConnectionDetails, client, pollIntervall) if errorPollEntity != nil { - log.Entry().WithError(errorPollEntity).Fatal("Pull of " + repositoryName + " failed on the ABAP System") + return errors.Wrapf(errorPollEntity, "Pull of '%s' failed on the ABAP System", repositoryName) } if status == "E" { - log.Entry().Fatal("Pull of " + repositoryName + " failed on the ABAP System") + return errors.New("Pull of " + repositoryName + " failed on the ABAP System") } log.Entry().Info(repositoryName + " was pulled successfully") diff --git a/cmd/abapEnvironmentPullGitRepo_test.go b/cmd/abapEnvironmentPullGitRepo_test.go index 00f725c43..7f0121fa2 100644 --- a/cmd/abapEnvironmentPullGitRepo_test.go +++ b/cmd/abapEnvironmentPullGitRepo_test.go @@ -15,6 +15,42 @@ import ( "github.com/stretchr/testify/assert" ) +func TestStep(t *testing.T) { + t.Run("Run Step Successful", func(t *testing.T) { + + var autils = abaputils.AUtilsMock{} + defer autils.Cleanup() + autils.ReturnedConnectionDetailsHTTP.Password = "password" + autils.ReturnedConnectionDetailsHTTP.User = "user" + autils.ReturnedConnectionDetailsHTTP.URL = "https://example.com" + autils.ReturnedConnectionDetailsHTTP.XCsrfToken = "xcsrftoken" + + config := abapEnvironmentPullGitRepoOptions{ + CfAPIEndpoint: "https://api.endpoint.com", + CfOrg: "testOrg", + CfSpace: "testSpace", + CfServiceInstance: "testInstance", + CfServiceKeyName: "testServiceKey", + Username: "testUser", + Password: "testPassword", + RepositoryNames: []string{"testRepo1"}, + } + + client := &clientMock{ + BodyList: []string{ + `{"d" : { "status" : "S" } }`, + `{"d" : { "status" : "S" } }`, + `{"d" : { "status" : "S" } }`, + }, + Token: "myToken", + StatusCode: 200, + } + + err := runAbapEnvironmentPullGitRepo(&config, nil, &autils, client) + assert.NoError(t, err, "Did not expect error") + }) +} + func TestTriggerPull(t *testing.T) { t.Run("Test trigger pull: success case", func(t *testing.T) { @@ -170,8 +206,11 @@ func TestGetAbapCommunicationArrangementInfo(t *testing.T) { } execRunner := &mock.ExecMockRunner{} + var autils = abaputils.AbapUtils{ + Exec: execRunner, + } - abaputils.GetAbapCommunicationArrangementInfo(options.AbapEnvOptions, execRunner, "/sap/opu/odata/sap/MANAGE_GIT_REPOSITORY/Pull") + autils.GetAbapCommunicationArrangementInfo(options.AbapEnvOptions, "/sap/opu/odata/sap/MANAGE_GIT_REPOSITORY/Pull") assert.Equal(t, "cf", execRunner.Calls[0].Exec, "Wrong command") assert.Equal(t, []string{"login", "-a", "https://api.endpoint.com", "-o", "testOrg", "-s", "testSpace", "-u", "testUser", "-p", "testPassword"}, execRunner.Calls[0].Params, "Wrong parameters") //assert.Equal(t, []string{"api", "https://api.endpoint.com"}, execRunner.Calls[0].Params, "Wrong parameters") @@ -195,8 +234,10 @@ func TestGetAbapCommunicationArrangementInfo(t *testing.T) { } execRunner := &mock.ExecMockRunner{} - - var _, err = abaputils.GetAbapCommunicationArrangementInfo(options.AbapEnvOptions, execRunner, "/sap/opu/odata/sap/MANAGE_GIT_REPOSITORY/Pull") + var autils = abaputils.AbapUtils{ + Exec: execRunner, + } + var _, err = autils.GetAbapCommunicationArrangementInfo(options.AbapEnvOptions, "/sap/opu/odata/sap/MANAGE_GIT_REPOSITORY/Pull") assert.Equal(t, "Parameters missing. Please provide EITHER the Host of the ABAP server OR the Cloud Foundry ApiEndpoint, Organization, Space, Service Instance and a corresponding Service Key for the Communication Scenario SAP_COM_0510", err.Error(), "Different error message expected") }) @@ -212,8 +253,10 @@ func TestGetAbapCommunicationArrangementInfo(t *testing.T) { } execRunner := &mock.ExecMockRunner{} - - var _, err = abaputils.GetAbapCommunicationArrangementInfo(options.AbapEnvOptions, execRunner, "/sap/opu/odata/sap/MANAGE_GIT_REPOSITORY/Pull") + var autils = abaputils.AbapUtils{ + Exec: execRunner, + } + var _, err = autils.GetAbapCommunicationArrangementInfo(options.AbapEnvOptions, "/sap/opu/odata/sap/MANAGE_GIT_REPOSITORY/Pull") assert.Equal(t, "Parameters missing. Please provide EITHER the Host of the ABAP server OR the Cloud Foundry ApiEndpoint, Organization, Space, Service Instance and a corresponding Service Key for the Communication Scenario SAP_COM_0510", err.Error(), "Different error message expected") }) diff --git a/cmd/abapEnvironmentRunATCCheck.go b/cmd/abapEnvironmentRunATCCheck.go index 4b70516fb..ef775637e 100644 --- a/cmd/abapEnvironmentRunATCCheck.go +++ b/cmd/abapEnvironmentRunATCCheck.go @@ -36,12 +36,15 @@ func abapEnvironmentRunATCCheck(options abapEnvironmentRunATCCheckOptions, telem subOptions.Password = options.Password subOptions.Username = options.Username - var c = &command.Command{} - var err error - + c := &command.Command{} c.Stdout(log.Entry().Writer()) c.Stderr(log.Entry().Writer()) + var autils = abaputils.AbapUtils{ + Exec: c, + } + var err error + client := piperhttp.Client{} cookieJar, _ := cookiejar.New(nil) clientOptions := piperhttp.ClientOptions{ @@ -53,7 +56,7 @@ func abapEnvironmentRunATCCheck(options abapEnvironmentRunATCCheckOptions, telem var abapEndpoint string //If Host flag is empty read ABAP endpoint from Service Key instead. Otherwise take ABAP system endpoint from config instead if err == nil { - details, err = abaputils.GetAbapCommunicationArrangementInfo(subOptions, c, "") + details, err = autils.GetAbapCommunicationArrangementInfo(subOptions, "") } var resp *http.Response //Fetch Xcrsf-Token diff --git a/cmd/abapEnvironmentRunATCCheck_test.go b/cmd/abapEnvironmentRunATCCheck_test.go index 41032c067..7220af08b 100644 --- a/cmd/abapEnvironmentRunATCCheck_test.go +++ b/cmd/abapEnvironmentRunATCCheck_test.go @@ -22,8 +22,11 @@ func TestHostConfig(t *testing.T) { } execRunner := &mock.ExecMockRunner{} + var autils = abaputils.AbapUtils{ + Exec: execRunner, + } var con abaputils.ConnectionDetailsHTTP - con, error := abaputils.GetAbapCommunicationArrangementInfo(options.AbapEnvOptions, execRunner, "") + con, error := autils.GetAbapCommunicationArrangementInfo(options.AbapEnvOptions, "") if error == nil { assert.Equal(t, "testUser", con.User) @@ -47,15 +50,18 @@ func TestHostConfig(t *testing.T) { } execRunner := &mock.ExecMockRunner{} + var autils = abaputils.AbapUtils{ + Exec: execRunner, + } - _, err := abaputils.GetAbapCommunicationArrangementInfo(options.AbapEnvOptions, execRunner, "") + _, err := autils.GetAbapCommunicationArrangementInfo(options.AbapEnvOptions, "") assert.EqualError(t, err, "Parameters missing. Please provide EITHER the Host of the ABAP server OR the Cloud Foundry ApiEndpoint, Organization, Space, Service Instance and a corresponding Service Key for the Communication Scenario SAP_COM_0510") //Testing without ABAP Host config = abaputils.AbapEnvironmentOptions{ Username: "testUser", Password: "testPassword", } - _, err = abaputils.GetAbapCommunicationArrangementInfo(options.AbapEnvOptions, execRunner, "") + _, err = autils.GetAbapCommunicationArrangementInfo(options.AbapEnvOptions, "") assert.EqualError(t, err, "Parameters missing. Please provide EITHER the Host of the ABAP server OR the Cloud Foundry ApiEndpoint, Organization, Space, Service Instance and a corresponding Service Key for the Communication Scenario SAP_COM_0510") }) @@ -73,8 +79,11 @@ func TestHostConfig(t *testing.T) { AbapEnvOptions: config, } execRunner := &mock.ExecMockRunner{} + var autils = abaputils.AbapUtils{ + Exec: execRunner, + } var con abaputils.ConnectionDetailsHTTP - con, error := abaputils.GetAbapCommunicationArrangementInfo(options.AbapEnvOptions, execRunner, "") + con, error := autils.GetAbapCommunicationArrangementInfo(options.AbapEnvOptions, "") if error == nil { assert.Equal(t, "", con.User) assert.Equal(t, "", con.Password) diff --git a/pkg/abaputils/abaputils.go b/pkg/abaputils/abaputils.go index bfec8cd34..96c19b212 100644 --- a/pkg/abaputils/abaputils.go +++ b/pkg/abaputils/abaputils.go @@ -10,9 +10,23 @@ import ( "github.com/pkg/errors" ) -// GetAbapCommunicationArrangementInfo function fetches the communcation arrangement information in SAP CP ABAP Environment -func GetAbapCommunicationArrangementInfo(options AbapEnvironmentOptions, c command.ExecRunner, oDataURL string) (ConnectionDetailsHTTP, error) { +/* +AbapUtils Struct +*/ +type AbapUtils struct { + Exec command.ExecRunner +} +/* +Communication for defining function used for communication +*/ +type Communication interface { + GetAbapCommunicationArrangementInfo(options AbapEnvironmentOptions, oDataURL string) (ConnectionDetailsHTTP, error) +} + +// GetAbapCommunicationArrangementInfo function fetches the communcation arrangement information in SAP CP ABAP Environment +func (abaputils *AbapUtils) GetAbapCommunicationArrangementInfo(options AbapEnvironmentOptions, oDataURL string) (ConnectionDetailsHTTP, error) { + c := abaputils.Exec var connectionDetails ConnectionDetailsHTTP var error error @@ -220,3 +234,20 @@ type AbapBinding struct { Version string `json:"version"` Env string `json:"env"` } + +// AUtilsMock mock +type AUtilsMock struct { + ReturnedConnectionDetailsHTTP ConnectionDetailsHTTP + ReturnedError error +} + +// GetAbapCommunicationArrangementInfo mock +func (autils *AUtilsMock) GetAbapCommunicationArrangementInfo(options AbapEnvironmentOptions, oDataURL string) (ConnectionDetailsHTTP, error) { + return autils.ReturnedConnectionDetailsHTTP, autils.ReturnedError +} + +// Cleanup to reset AUtilsMock +func (autils *AUtilsMock) Cleanup() { + autils.ReturnedConnectionDetailsHTTP = ConnectionDetailsHTTP{} + autils.ReturnedError = nil +} diff --git a/pkg/abaputils/abaputils_test.go b/pkg/abaputils/abaputils_test.go index d70ce9c05..eed9f8c64 100644 --- a/pkg/abaputils/abaputils_test.go +++ b/pkg/abaputils/abaputils_test.go @@ -25,7 +25,10 @@ func TestCloudFoundryGetAbapCommunicationInfo(t *testing.T) { //when var connectionDetails ConnectionDetailsHTTP var err error - connectionDetails, err = GetAbapCommunicationArrangementInfo(options, &command.Command{}, "") + var autils = AbapUtils{ + Exec: &command.Command{}, + } + connectionDetails, err = autils.GetAbapCommunicationArrangementInfo(options, "") //then assert.Equal(t, "", connectionDetails.URL) @@ -52,7 +55,10 @@ func TestCloudFoundryGetAbapCommunicationInfo(t *testing.T) { //when var connectionDetails ConnectionDetailsHTTP var err error - connectionDetails, err = GetAbapCommunicationArrangementInfo(options, &command.Command{}, "") + var autils = AbapUtils{ + Exec: &command.Command{}, + } + connectionDetails, err = autils.GetAbapCommunicationArrangementInfo(options, "") //then assert.Equal(t, "", connectionDetails.URL) @@ -65,7 +71,6 @@ func TestCloudFoundryGetAbapCommunicationInfo(t *testing.T) { t.Run("CF GetAbapCommunicationArrangementInfo - Success", func(t *testing.T) { //given - m := &mock.ExecMockRunner{} const testURL = "https://testurl.com" const oDataURL = "/sap/opu/odata/sap/MANAGE_GIT_REPOSITORY/Pull" @@ -85,12 +90,15 @@ func TestCloudFoundryGetAbapCommunicationInfo(t *testing.T) { CfServiceKeyName: "testServiceKeyName", } + m := &mock.ExecMockRunner{} m.StdoutReturn = map[string]string{"cf service-key testInstance testServiceKeyName": serviceKey} - + var autils = AbapUtils{ + Exec: m, + } //when var connectionDetails ConnectionDetailsHTTP var err error - connectionDetails, err = GetAbapCommunicationArrangementInfo(options, m, oDataURL) + connectionDetails, err = autils.GetAbapCommunicationArrangementInfo(options, oDataURL) //then assert.Equal(t, testURL+oDataURL, connectionDetails.URL) @@ -105,7 +113,6 @@ func TestHostGetAbapCommunicationInfo(t *testing.T) { t.Run("HOST GetAbapCommunicationArrangementInfo - Success", func(t *testing.T) { //given - m := &mock.ExecMockRunner{} const testURL = "https://testurl.com" const oDataURL = "/sap/opu/odata/sap/MANAGE_GIT_REPOSITORY/Pull" @@ -121,12 +128,16 @@ func TestHostGetAbapCommunicationInfo(t *testing.T) { Password: password, } + m := &mock.ExecMockRunner{} m.StdoutReturn = map[string]string{"cf service-key testInstance testServiceKeyName": serviceKey} + var autils = AbapUtils{ + Exec: m, + } //when var connectionDetails ConnectionDetailsHTTP var err error - connectionDetails, err = GetAbapCommunicationArrangementInfo(options, m, oDataURL) + connectionDetails, err = autils.GetAbapCommunicationArrangementInfo(options, oDataURL) //then assert.Equal(t, testURL+oDataURL, connectionDetails.URL) @@ -139,7 +150,6 @@ func TestHostGetAbapCommunicationInfo(t *testing.T) { t.Run("HOST GetAbapCommunicationArrangementInfo - Success - w/o https", func(t *testing.T) { //given - m := &mock.ExecMockRunner{} const testURL = "testurl.com" const oDataURL = "/sap/opu/odata/sap/MANAGE_GIT_REPOSITORY/Pull" @@ -155,12 +165,16 @@ func TestHostGetAbapCommunicationInfo(t *testing.T) { Password: password, } + m := &mock.ExecMockRunner{} m.StdoutReturn = map[string]string{"cf service-key testInstance testServiceKeyName": serviceKey} + var autils = AbapUtils{ + Exec: m, + } //when var connectionDetails ConnectionDetailsHTTP var err error - connectionDetails, err = GetAbapCommunicationArrangementInfo(options, m, oDataURL) + connectionDetails, err = autils.GetAbapCommunicationArrangementInfo(options, oDataURL) //then assert.Equal(t, "https://"+testURL+oDataURL, connectionDetails.URL)