Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: Test the container package. #884

Closed
HarikrishnanBalagopal opened this issue Oct 2, 2022 · 5 comments
Closed

test: Test the container package. #884

HarikrishnanBalagopal opened this issue Oct 2, 2022 · 5 comments
Assignees
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. hacktoberfest help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.

Comments

@HarikrishnanBalagopal
Copy link
Contributor

⚠️ Don't forget to read our contribution guidelines before you start working on an issue https://github.com/konveyor/move2kube/blob/main/contributing.md

Description

Since Move2Kube moved from v1 to v2 and then to the v3 architecture, the test coverage has dropped significantly.
One of the many places that need unit tests are the files in the container package.
It is a core part of Move2Kube's transform functionality and yet has only a few unit tests.

https://github.com/konveyor/move2kube/blob/6ee8f6be3e7f9b9eb0bb1200b468ac95018f244f/environment/container

This package deals with pulling container images, starting and stopping containers, etc. using different runtimes like Docker and Podman.
It can also copy files/directories from the local filesystem into the container and vice versa.

https://github.com/konveyor/move2kube/blob/6ee8f6be3e7f9b9eb0bb1200b468ac95018f244f/environment/container/utils.go

How to get started

For this issue you should start with some simple tests that pulls a container image, starts a container, copies a file into the container, etc.
Next step would be to make slight changes to the files in the directory and create a separate subtest for each scenario.
Some scenarios that are good to test:

  • Detecting if a container runtime is installed and which one (Docker/Podman).
  • Pulling a container image.
  • Starting a container using the image.
  • Copying a normal file into a container.
  • Copying a normal directory into a container.
  • Copying a normal directory from the container to the local filesystem.
  • Copy function called on an empty directory.
  • Directory we don't have permission to read.
  • Directory with several subdirectories

How to add unit tests

Some guidelines:

  • Look at the other tests in the package/project and write similar tests. Example:
    func TestIsBuilderAvailable(t *testing.T) {
    logrus.SetLevel(logrus.DebugLevel)
    t.Run("normal use case", func(t *testing.T) {
    provider, _ := newDockerEngine()
    image := "quay.io/konveyor/move2kube"
    // Test
    if err := provider.pullImage(image); err != nil {
    t.Fatalf("Failed to find the image '%s' locally and/or pull it. Error: %q", image, err)
    }
    })
    t.Run("normal use case where we get result from cache", func(t *testing.T) {
    provider, _ := newDockerEngine()
    image := "quay.io/konveyor/move2kube"
    // Test
    if err := provider.pullImage(image); err != nil {
    t.Fatalf("Failed to find the image '%s' locally and/or pull it. Error: %q", image, err)
    }
    if !provider.availableImages[image] {
    t.Fatalf("Failed to add the image %q to the list of available images", image)
    }
    if err := provider.pullImage(image); err != nil {
    t.Fatalf("Failed to find the image '%s' locally and/or pull it. Error: %q", image, err)
    }
    })
    t.Run("check for a non existent image", func(t *testing.T) {
    provider, _ := newDockerEngine()
    image := "this/doesnotexist:foobar"
    if err := provider.pullImage(image); err == nil {
    t.Fatalf("Should not have succeeded. The image '%s' does not exist", image)
    }
    })
    }

    func TestMerge(t *testing.T) {
    t.Run("merging 2 empty metadatas", func(t *testing.T) {
    cmeta1 := collection.NewClusterMetadata("")
    cmeta1.Kind = ""
    cmeta2 := collection.NewClusterMetadata("")
    cmeta2.Kind = ""
    want := collection.NewClusterMetadata("")
    want.Kind = ""
    if merged := cmeta1.Merge(cmeta2); !merged || !reflect.DeepEqual(cmeta1, want) {
    t.Fatalf("Failed to merge ClusterMetadata properly. Difference:\n%s:", cmp.Diff(want, cmeta1))
    }
    })
    t.Run("merging a non empty metadata into an empty metadata", func(t *testing.T) {
    cmeta1 := collection.NewClusterMetadata("")
    cmeta1.Kind = ""
    cmeta2 := collection.NewClusterMetadata("ctxname1")
    want := collection.NewClusterMetadata("")
    want.Name = "ctxname1"
    want.Spec.StorageClasses = []string{"default"}
    if merged := cmeta1.Merge(cmeta2); !merged || !reflect.DeepEqual(cmeta1, want) {
    t.Fatalf("Failed to merge ClusterMetadata properly. Difference:\n%s:", cmp.Diff(want, cmeta1))
    }
    })
    t.Run("merging metadata with different kinds", func(t *testing.T) {
    cmeta1 := collection.NewClusterMetadata("")
    cmeta1.Kind = "kind1"
    cmeta2 := collection.NewClusterMetadata("")
    cmeta2.Kind = "kind2"
    if merged := cmeta1.Merge(cmeta2); merged {
    t.Fatal("Should not have merged metadata with different kinds. The kinds are", cmeta1.Kind, "and", cmeta2.Kind)
    }
    })
    t.Run("merging version maps from filled metadata into filled metadata", func(t *testing.T) {
    key1 := "key1"
    val1 := []string{"1.0.0", "1.1.0", "1.1.1"}
    key2 := "key2"
    val2 := []string{"2.0.0", "2.2.0", "2.2.2"}
    cmeta1 := collection.NewClusterMetadata("")
    cmeta1.Spec.APIKindVersionMap = map[string][]string{key1: val1}
    cmeta2 := collection.NewClusterMetadata("")
    cmeta2.Spec.APIKindVersionMap = map[string][]string{key1: val2, key2: val2}
    cmeta2.Spec.Host = "host"
    want := collection.NewClusterMetadata("")
    want.Spec.StorageClasses = []string{"default"}
    want.Spec.APIKindVersionMap = map[string][]string{key1: val2}
    want.Spec.Host = "host"
    if merged := cmeta1.Merge(cmeta2); !merged || !reflect.DeepEqual(cmeta1, want) {
    t.Fatalf("Failed to merge ClusterMetadata properly. Difference:\n%s:", cmp.Diff(want, cmeta1))
    }
    })
    t.Run("merging storage classes from filled metadata into filled metadata", func(t *testing.T) {
    cmeta1 := collection.NewClusterMetadata("")
    cmeta1.Spec.StorageClasses = []string{"111", "222", "333"}
    cmeta2 := collection.NewClusterMetadata("")
    cmeta2.Spec.StorageClasses = []string{"222", "333", "444"}
    want := collection.NewClusterMetadata("")
    want.Spec.StorageClasses = []string{"222", "333"}
    if merged := cmeta1.Merge(cmeta2); !merged || !reflect.DeepEqual(cmeta1, want) {
    t.Fatalf("Failed to merge ClusterMetadata properly. Difference:\n%s:", cmp.Diff(want, cmeta1))
    }
    })
    }
  • Use subtests to test different paths through the function.
  • Don't worry about testing every single path through the function. Focus on common use cases and failure modes.

Some helpful resources on how to write unit tests in Go:

Code to be tested

https://github.com/konveyor/move2kube/blob/6ee8f6be3e7f9b9eb0bb1200b468ac95018f244f/environment/container

@HarikrishnanBalagopal HarikrishnanBalagopal added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. hacktoberfest labels Oct 2, 2022
@moki1202
Copy link

moki1202 commented Feb 13, 2023

@HarikrishnanBalagopal I've written a few (rather simple) go tests so I think I can try to give this a go :)

@punithnayak
Copy link

Hey @HarikrishnanBalagopal Can you assign me so that can raise a Pr

@HarikrishnanBalagopal
Copy link
Contributor Author

Hey @HarikrishnanBalagopal Can you assign me so that can raise a Pr

Thanks for your interest. I have assigned the issue to you. Feel free to ask in the #konveyor Slack channel to get help https://kubernetes.slack.com/archives/CR85S82A2

@akagami-harsh
Copy link
Contributor

hey @HarikrishnanBalagopal, what's the status on the issue

@HarikrishnanBalagopal
Copy link
Contributor Author

HarikrishnanBalagopal commented Mar 21, 2024

@akagami-harsh Fixed in #994 and #997

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. hacktoberfest help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.
Projects
Status: Done
Development

No branches or pull requests

4 participants