Provide integration tests via the key Ditto features - SoftwareUpdatable by antonia-avramova · Pull Request #182 · eclipse-kanto/container-management

antonia-avramova · 2023-08-02T13:08:11Z

[#65] Provide integration tests via the key Ditto features - SoftwareUpdatable

Signed-off-by: Antonia Avramova [email protected]

Signed-off-by: Antonia Avramova <[email protected]>

k-gostev

Overall looks very good, I have just a couple of nits

k-gostev · 2023-08-04T06:42:59Z

integration/ctr_software_updatable_test.go

+	"testing"
+
+	"os"
+	"strings"
+
+	"github.com/eclipse-kanto/container-management/rollouts/api/datatypes"
+	"github.com/eclipse-kanto/kanto/integration/util"
+	"github.com/eclipse/ditto-clients-golang/protocol"
+	"github.com/google/uuid"
+
+	"github.com/stretchr/testify/require"
+	"github.com/stretchr/testify/suite"


Suggested change

"testing"

"os"

"strings"

"github.com/eclipse-kanto/container-management/rollouts/api/datatypes"

"github.com/eclipse-kanto/kanto/integration/util"

"github.com/eclipse/ditto-clients-golang/protocol"

"github.com/google/uuid"

"github.com/stretchr/testify/require"

"github.com/stretchr/testify/suite"

"testing"

"os"

"strings"

"github.com/eclipse-kanto/container-management/rollouts/api/datatypes"

"github.com/eclipse-kanto/kanto/integration/util"

"github.com/eclipse/ditto-clients-golang/protocol"

"github.com/google/uuid"

"github.com/stretchr/testify/require"

"github.com/stretchr/testify/suite"

k-gostev · 2023-08-04T06:46:39Z

integration/ctr_software_updatable_test.go

+	actionInstall = "install"
+	paramCorID    = "correlationId"
+	paramForced   = "forced"
+	validJSONUrl  = "https://raw.githubusercontent.com/eclipse-kanto/container-management/main/containerm/pkg/testutil/config/container/valid.json"


As per https://github.com/golang/go/wiki/CodeReviewComments#initialisms

Suggested change

validJSONUrl = "https://raw.githubusercontent.com/eclipse-kanto/container-management/main/containerm/pkg/testutil/config/container/valid.json"

validContainerURL = "https://raw.githubusercontent.com/eclipse-kanto/container-management/main/containerm/pkg/testutil/config/container/valid.json"

k-gostev · 2023-08-04T06:49:47Z

integration/ctr_software_updatable_test.go

+}
+
+const (
+	suFeatureID   = "SoftwareUpdatable"


Please use this constant instead

k-gostev · 2023-08-04T06:50:34Z

integration/ctr_software_updatable_test.go

+
+	suite.swUpdatableFeatureURL = util.GetFeatureURL(suite.ctrThingURL, suFeatureID)
+
+	suite.assertCtrFeatureDefinition(suite.swUpdatableFeatureURL, "[\"org.eclipse.hawkbit.swupdatable:SoftwareUpdatable:2.0.0\"]")


Please export the necessary constants from here and reuse them.

k-gostev · 2023-08-04T06:53:44Z

integration/ctr_software_updatable_test.go

+	filter := fmt.Sprintf("like(resource:path,'/features/%s*')", suFeatureID)
+
+	err := util.SubscribeForWSMessages(suite.Cfg, wsConnection, util.StartSendEvents, filter)


Suggested change

filter := fmt.Sprintf("like(resource:path,'/features/%s*')", suFeatureID)

err := util.SubscribeForWSMessages(suite.Cfg, wsConnection, util.StartSendEvents, filter)

err := util.SubscribeForWSMessages(suite.Cfg, wsConnection, util.StartSendEvents, fmt.Sprintf("like(resource:path,'/features/%s*')", suFeatureID)

k-gostev · 2023-08-04T07:00:04Z

integration/ctr_software_updatable_test.go

+	err = util.ProcessWSMessages(suite.Cfg, wsConnection, func(event *protocol.Envelope) (bool, error) {
+		var err error
+		if event.Topic.String() == suite.topicModified {
+			if event.Path == fmt.Sprintf("/features/%s/properties/status/lastOperation", suFeatureID) {


/features/%s/properties/status/ is quite repetitive, could you create a function constructStatusPath(featureID, property string) that constructs the status and use it instead.

k-gostev · 2023-08-04T07:00:37Z

integration/ctr_software_updatable_test.go

+	err = suite.removeContainer(createRemoveParams(ctrID))
+	require.NoError(suite.T(), err, "failed to process removing the container")


Suggested change

err = suite.removeContainer(createRemoveParams(ctrID))

require.NoError(suite.T(), err, "failed to process removing the container")

require.NoError(suite.T(), suite.removeContainer(createRemoveParams(ctrID)), "failed to process removing the container")

k-gostev · 2023-08-04T07:01:31Z

integration/ctr_software_updatable_test.go

+	err := suite.removeContainer(createRemoveParams(ctrID))
+	require.ErrorContains(suite.T(), err, "container with ID = "+ctrID+" does not exist")


Suggested change

err := suite.removeContainer(createRemoveParams(ctrID))

require.ErrorContains(suite.T(), err, "container with ID = "+ctrID+" does not exist")

require.ErrorContains(suite.T(), suite.removeContainer(createRemoveParams(ctrID)), "container with ID = "+ctrID+" does not exist")

k-gostev · 2023-08-04T07:04:30Z

integration/ctr_software_updatable_test.go

+	err = os.WriteFile(filePath, src, 7777)
+	require.NoError(suite.T(), err, "unable to write file with path "+filePath)
+
+	params := createInstallParams(filePath, src, wrongChecksum)
+	err = os.Remove(filePath)
+	require.NoError(suite.T(), err, "unable to remove file with path "+filePath)
+	return params


could you try this way?

Suggested change

err = os.WriteFile(filePath, src, 7777)

require.NoError(suite.T(), err, "unable to write file with path "+filePath)

params := createInstallParams(filePath, src, wrongChecksum)

err = os.Remove(filePath)

require.NoError(suite.T(), err, "unable to remove file with path "+filePath)

return params

require.NoError(suite.T(), os.WriteFile(filePath, src, 7777), "unable to write file with path "+filePath)

defer require.NoError(suite.T(), os.Remove(filePath), "unable to remove file with path "+filePath)

return createInstallParams(filePath, src, wrongChecksum)

k-gostev · 2023-08-04T07:13:33Z

integration/ctr_software_updatable_test.go

+	err = json.Unmarshal(bytes, &ctrImage)
+	return fileStat, ctrImage, err


Suggested change

err = json.Unmarshal(bytes, &ctrImage)

return fileStat, ctrImage, err

err =

return fileStat, ctrImage, json.Unmarshal(bytes, &ctrImage)

Signed-off-by: Antonia Avramova <[email protected]>

k-gostev · 2023-08-07T07:45:09Z

integration/ctr_software_updatable_test.go

+		"software": []*datatypes.DependencyDescription{
+			&datatypes.DependencyDescription{
+				Name: ctrID,
+			},
+		},
+	}


Suggested change

"software": []*datatypes.DependencyDescription{

&datatypes.DependencyDescription{

Name: ctrID,

},

},

}

"software": []*datatypes.DependencyDescription{

{

Name: ctrID,

},

},

}

k-gostev · 2023-08-07T08:51:10Z

integration/ctr_software_updatable_test.go

+	require.NoError(suite.T(), err, "failed to subscribe for the %s messages", util.StartSendEvents)
+
+	_, err = util.ExecuteOperation(suite.Cfg, suite.suFeatureURL, "remove", params)
+	suite.closeOnError(wsConnection, err, "failed to execute software updatable install for containers with params %v", params)


Suggested change

suite.closeOnError(wsConnection, err, "failed to execute software updatable install for containers with params %v", params)

suite.closeOnError(wsConnection, err, "failed to execute software updatable remove for containers with params %v", params)

k-gostev · 2023-08-07T10:11:11Z

integration/ctr_software_updatable_test.go

+		wsConnection.Close()
+	}
+	require.Errorf(suite.T(), err, "failed to execute software updatable install for containers with params %v", params)
+


Suggested change

k-gostev · 2023-08-07T10:11:48Z

integration/ctr_software_updatable_test.go

+	if err != nil {
+		return nil, err
+	}
+	defer response.Body.Close()


Suggested change

if err != nil {

return nil, err

}

defer response.Body.Close()

if err != nil {

return nil, err

}

if response.StatusCode < http.StatusOK || response.StatusCode >= http.StatusMultipleChoices {

return nil, fmt.Errorf("http status code is not in the 2xx range: %v", response.StatusCode)

}

defer response.Body.Close()

k-gostev · 2023-08-07T10:19:53Z

integration/ctr_software_updatable_test.go

+func (suite *softwareUpdatableSuite) createParameters(wrongChecksum bool) map[string]interface{} {
+	src, err := download(validContainerURL)
+	require.NoError(suite.T(), err, "unable to download file from url "+validContainerURL)
+	splitStr := strings.Split(validContainerURL, "/")
+	filePath := "/tmp/" + splitStr[len(splitStr)-1]
+	require.NoError(suite.T(), os.WriteFile(filePath, src, 7777), "unable to write file with path "+filePath)
+
+	defer require.NoError(suite.T(), os.Remove(filePath), "unable to remove file with path "+filePath)
+	return createInstallParams(filePath, src, wrongChecksum)
+}
+
+func createInstallParams(filePath string, src []byte, wrongChecksum bool) map[string]interface{} {
+	fileInfo, ctrStruct, err := getCtrImageStructure(filePath)
+	if err != nil {
+		return make(map[string]interface{})
+	}


In case of some error related to the file operations the test will fail in a not a user friendly manner

Suggested change

func (suite *softwareUpdatableSuite) createParameters(wrongChecksum bool) map[string]interface{} {

src, err := download(validContainerURL)

require.NoError(suite.T(), err, "unable to download file from url "+validContainerURL)

splitStr := strings.Split(validContainerURL, "/")

filePath := "/tmp/" + splitStr[len(splitStr)-1]

require.NoError(suite.T(), os.WriteFile(filePath, src, 7777), "unable to write file with path "+filePath)

defer require.NoError(suite.T(), os.Remove(filePath), "unable to remove file with path "+filePath)

return createInstallParams(filePath, src, wrongChecksum)

}

func createInstallParams(filePath string, src []byte, wrongChecksum bool) map[string]interface{} {

fileInfo, ctrStruct, err := getCtrImageStructure(filePath)

if err != nil {

return make(map[string]interface{})

}

func (suite *softwareUpdatableSuite) createParameters(wrongChecksum bool) map[string]interface{} {

src, err := download(validContainerURL)

require.NoError(suite.T(), err, fmt.Sprintf("unable to download file from url %s", validContainerURL))

filePath := "valid.json"

require.NoError(suite.T(), os.WriteFile(filePath, src, 0755), "unable to write file with path "+filePath)

fileInfo, ctrStruct, err := getCtrImageStructure(filePath)

require.NoError(suite.T(), err, "unable to create container image struct")

defer require.NoError(suite.T(), os.Remove(filePath), "unable to remove file with path "+filePath)

return createInstallParams(fileInfo, ctrStruct, src, wrongChecksum)

}

func createInstallParams(fileInfo fs.FileInfo, ctrStruct CtrImageStruct, src []byte, wrongChecksum bool) map[string]interface{} {

splitStr := strings.Split(ctrStruct.Image.Name, "/")

swModuleStr := strings.Split(splitStr[len(splitStr)-1], ":")

Signed-off-by: Antonia Avramova <[email protected]>

Provide integration tests via the key Ditto features - SoftwareUpdatable

7210110

Signed-off-by: Antonia Avramova <[email protected]>

k-gostev linked an issue Aug 4, 2023 that may be closed by this pull request

Provide integration tests via the key Ditto features - SoftwareUpdatable #65

Closed

k-gostev self-requested a review August 4, 2023 06:42

k-gostev suggested changes Aug 4, 2023

View reviewed changes

antonia-avramova added 2 commits August 4, 2023 15:07

Provide integration tests via the key Ditto features - SoftwareUpdatable

6c534a0

Signed-off-by: Antonia Avramova <[email protected]>

Provide integration tests via the key Ditto features - SoftwareUpdatable

7de3773

Signed-off-by: Antonia Avramova <[email protected]>

k-gostev reviewed Aug 7, 2023

View reviewed changes

Provide integration tests via the key Ditto features - SoftwareUpdatable

fc4bc49

Signed-off-by: Antonia Avramova <[email protected]>

k-gostev approved these changes Aug 10, 2023

View reviewed changes

k-gostev merged commit b0e35dc into eclipse-kanto:main Aug 10, 2023

k-gostev deleted the su-integration-test branch August 10, 2023 11:08

	validJSONUrl = "https://raw.githubusercontent.com/eclipse-kanto/container-management/main/containerm/pkg/testutil/config/container/valid.json"
	validContainerURL = "https://raw.githubusercontent.com/eclipse-kanto/container-management/main/containerm/pkg/testutil/config/container/valid.json"


		suite.swUpdatableFeatureURL = util.GetFeatureURL(suite.ctrThingURL, suFeatureID)

		suite.assertCtrFeatureDefinition(suite.swUpdatableFeatureURL, "[\"org.eclipse.hawkbit.swupdatable:SoftwareUpdatable:2.0.0\"]")

		filter := fmt.Sprintf("like(resource:path,'/features/%s*')", suFeatureID)

		err := util.SubscribeForWSMessages(suite.Cfg, wsConnection, util.StartSendEvents, filter)

		err = suite.removeContainer(createRemoveParams(ctrID))
		require.NoError(suite.T(), err, "failed to process removing the container")

	err = suite.removeContainer(createRemoveParams(ctrID))
	require.NoError(suite.T(), err, "failed to process removing the container")
	require.NoError(suite.T(), suite.removeContainer(createRemoveParams(ctrID)), "failed to process removing the container")

		err := suite.removeContainer(createRemoveParams(ctrID))
		require.ErrorContains(suite.T(), err, "container with ID = "+ctrID+" does not exist")

	err := suite.removeContainer(createRemoveParams(ctrID))
	require.ErrorContains(suite.T(), err, "container with ID = "+ctrID+" does not exist")
	require.ErrorContains(suite.T(), suite.removeContainer(createRemoveParams(ctrID)), "container with ID = "+ctrID+" does not exist")

		err = json.Unmarshal(bytes, &ctrImage)
		return fileStat, ctrImage, err

	suite.closeOnError(wsConnection, err, "failed to execute software updatable install for containers with params %v", params)
	suite.closeOnError(wsConnection, err, "failed to execute software updatable remove for containers with params %v", params)

Conversation

antonia-avramova commented Aug 2, 2023

Uh oh!

k-gostev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k-gostev Aug 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

k-gostev Aug 4, 2023 •

edited

Loading