Adding tests for `kedge build` #381

ashetty1 · 2017-10-23T12:51:28Z

The test here creates a local docker registry, runs kedge build, and verifies if the image has been uploaded.

ashetty1 · 2017-10-23T12:51:52Z

@surajssd would like to have your inputs on this.

ashetty1 · 2017-10-23T13:19:27Z

Working on fixing the travis errors

surajssd

So this approach LGTM!

Start registry
Login to registry
Do kedge build/push
Verify that image is built locally
Verify that image is available on the registry
Clean registry and image

surajssd · 2017-10-25T08:29:04Z

tests/e2e/build_test.go

+	_, err = runCmd("docker rm -v kedge_registry")
+	if err != nil {
+		t.Error(err)
+	}


put all of the clean up code in a fucntion and then call it with defer so that we always do the clean up!

surajssd · 2017-11-20T05:25:50Z

@ashetty1 tried running it, but getting following error, I think this should also be taken care!

$ go test -run TestBuild github.com/kedgeproject/kedge/tests/e2e
--- FAIL: TestBuild (0.01s)
panic: error running command &{/bin/sh [/bin/sh -c docker run -d -p 5000:5000 --restart=always --name kedge_registry registry:2] []  <nil>  docker: Error response from daemon: Conflict. The container name "/kedge_registry" is already in use by container "889c30df6d0c704c862790f8a6ef489c5e9a3b93638aadf57ce301d402e914d9". You have to remove (or rename) that container to be able to reuse that name.
See 'docker run --help'.
 [] <nil> 0xc4201a0930 exit status 125 <nil> <nil> true [0xc420302098 0xc4203020b8 0xc4203020d0] [0xc420302098 0xc4203020b8 0xc4203020d0] [0xc4203020a8 0xc4203020c8] [0x552540 0x552540] 0xc4201ce660 <nil>}: exit status 125 [recovered]
        panic: error running command &{/bin/sh [/bin/sh -c docker run -d -p 5000:5000 --restart=always --name kedge_registry registry:2] []  <nil>  docker: Error response from daemon: Conflict. The container name "/kedge_registry" is already in use by container "889c30df6d0c704c862790f8a6ef489c5e9a3b93638aadf57ce301d402e914d9". You have to remove (or rename) that container to be able to reuse that name.
See 'docker run --help'.
 [] <nil> 0xc4201a0930 exit status 125 <nil> <nil> true [0xc420302098 0xc4203020b8 0xc4203020d0] [0xc420302098 0xc4203020b8 0xc4203020d0] [0xc4203020a8 0xc4203020c8] [0x552540 0x552540] 0xc4201ce660 <nil>}: exit status 125

goroutine 20 [running]:
testing.tRunner.func1(0xc4202b60f0)
        /usr/local/go/src/testing/testing.go:711 +0x2d2
panic(0xfd1000, 0xc42044a080)
        /usr/local/go/src/runtime/panic.go:491 +0x283
github.com/kedgeproject/kedge/tests/e2e.TestBuild(0xc4202b60f0)
        /home/fedora/go/src/github.com/kedgeproject/kedge/tests/e2e/build_test.go:60 +0x80e
testing.tRunner(0xc4202b60f0, 0x11c3820)
        /usr/local/go/src/testing/testing.go:746 +0xd0
created by testing.(*T).Run
        /usr/local/go/src/testing/testing.go:789 +0x2de
FAIL    github.com/kedgeproject/kedge/tests/e2e 0.068s

surajssd · 2017-11-20T05:35:46Z

After cleaning up manually

$ docker stop kedge_registry
$ docker rm -f $(docker ps -aq)

I ran that individual test and it took almost 4mins to complete:

$ go test -run TestBuild github.com/kedgeproject/kedge/tests/e2e                                                                                                                        
ok      github.com/kedgeproject/kedge/tests/e2e 231.089s

This should run in parallel with e2e tests, to make it faster.

surajssd · 2017-11-20T05:43:02Z

Also it is using the docker env of the host, not of the minikube or minishift vm!

ashetty1 · 2017-11-28T07:01:17Z

@surajssd I am not sure if we need to check if there is a docker registry with the same name before running the test. What we should expect before the test is a clean setup and then leave the system as it was once the test run is complete

surajssd · 2017-11-28T12:54:15Z

tests/e2e/build_test.go

+		t.Error(err)
+	}
+
+	dockerRegStart := "docker run -d -p 5000:5000 --restart=always --name kedge_registry registry:2"


You can also add the flag --rm which will delete the docker container once the registry is stopped!

This will reduce the whole hassle of deleting the container

surajssd · 2017-11-28T12:55:33Z

@surajssd I am not sure if we need to check if there is a docker registry with the same name before running the test. What we should expect before the test is a clean setup and then leave the system as it was once the test run is complete

why should we assume that? We will also be running tests locally so we don't want to leave the environment dirty in dev machine!

ashetty1 · 2017-12-04T07:53:44Z

@surajssd kindly review this.

surajssd

LGTM!

cdrage

Would be nice to add code comments 👍

cdrage · 2017-12-05T14:26:18Z

tests/e2e/build_test.go

+
+	defer DeleteRegistry(t)
+
+	runKedgeCmd := KedgeLoc + "/kedge build -c " + contextDir + " -i localhost:5000/test:2.0 -p"


have a look at https://github.com/kedgeproject/kedge/blob/master/tests/e2e/e2e_test.go#L30

would be better to throw the binary location up in global variables

cdrage · 2017-12-05T14:27:31Z

tests/e2e/build_test.go

+	var contextDir = KedgeLoc + "/docs/examples/build"
+
+	// Cleanup
+	_, _ = runCmd("docker stop kedge_registry")


consider adding a slower timeout -t 1 in order to speed up tests.

cdrage · 2017-12-05T14:27:46Z

tests/e2e/build_test.go

+		t.Error(err)
+	}
+
+	dockerRegStart := "docker run -d -p 5000:5000 --restart=always --name kedge_registry registry:2"


The test here creates a local docker registry, runs `kedge build`, and verifies if the image has been uploaded

ashetty1 · 2017-12-07T11:32:07Z

@cdrage could you please do a final review here.

cdrage · 2017-12-12T19:22:52Z

This LGTM 👍 merge when you're comfortable.

ashetty1 · 2017-12-13T07:15:45Z

@surajssd @kadel let me know if you guys think this can be merged.

kadel · 2017-12-13T12:08:28Z

this already has two LGTMs.
I'm merging this

ashetty1 force-pushed the build_test branch from cc4cc02 to 31e8125 Compare October 23, 2017 13:07

ashetty1 force-pushed the build_test branch 3 times, most recently from 9c427ee to c925569 Compare October 24, 2017 09:22

surajssd suggested changes Oct 25, 2017

View reviewed changes

ashetty1 force-pushed the build_test branch from c925569 to a07be31 Compare October 26, 2017 08:27

ashetty1 changed the title ~~[WIP] Adding tests for kedge build~~ Adding tests for kedge build Oct 26, 2017

cdrage force-pushed the master branch 2 times, most recently from 52766ab to 388b72b Compare November 2, 2017 22:59

surajssd reviewed Nov 28, 2017

View reviewed changes

ashetty1 force-pushed the build_test branch from a07be31 to d354a32 Compare November 29, 2017 09:27

surajssd approved these changes Dec 5, 2017

View reviewed changes

cdrage suggested changes Dec 5, 2017

View reviewed changes

Adding tests for kedge build:

1f9f576

The test here creates a local docker registry, runs `kedge build`, and verifies if the image has been uploaded

ashetty1 force-pushed the build_test branch from d354a32 to 1f9f576 Compare December 6, 2017 11:41

cdrage approved these changes Dec 12, 2017

View reviewed changes

ashetty1 mentioned this pull request Dec 13, 2017

Test the kedge build functionality #308

Closed

2 tasks

kadel merged commit f07524d into kedgeproject:master Dec 13, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding tests for `kedge build` #381

Adding tests for `kedge build` #381

ashetty1 commented Oct 23, 2017 •

edited

ashetty1 commented Oct 23, 2017

ashetty1 commented Oct 23, 2017

surajssd left a comment

surajssd Oct 25, 2017

surajssd commented Nov 20, 2017

surajssd commented Nov 20, 2017

surajssd commented Nov 20, 2017

ashetty1 commented Nov 28, 2017

surajssd Nov 28, 2017

surajssd Nov 28, 2017

cdrage Dec 5, 2017

surajssd commented Nov 28, 2017

ashetty1 commented Dec 4, 2017

surajssd left a comment

cdrage left a comment

cdrage Dec 5, 2017

cdrage Dec 5, 2017

cdrage Dec 5, 2017

ashetty1 commented Dec 7, 2017

cdrage commented Dec 12, 2017

ashetty1 commented Dec 13, 2017

kadel commented Dec 13, 2017


		defer DeleteRegistry(t)

		runKedgeCmd := KedgeLoc + "/kedge build -c " + contextDir + " -i localhost:5000/test:2.0 -p"

Adding tests for kedge build #381

Adding tests for kedge build #381

Conversation

ashetty1 commented Oct 23, 2017 • edited

ashetty1 commented Oct 23, 2017

ashetty1 commented Oct 23, 2017

surajssd left a comment

Choose a reason for hiding this comment

surajssd Oct 25, 2017

Choose a reason for hiding this comment

surajssd commented Nov 20, 2017

surajssd commented Nov 20, 2017

surajssd commented Nov 20, 2017

ashetty1 commented Nov 28, 2017

surajssd Nov 28, 2017

Choose a reason for hiding this comment

surajssd Nov 28, 2017

Choose a reason for hiding this comment

cdrage Dec 5, 2017

Choose a reason for hiding this comment

surajssd commented Nov 28, 2017

ashetty1 commented Dec 4, 2017

surajssd left a comment

Choose a reason for hiding this comment

cdrage left a comment

Choose a reason for hiding this comment

cdrage Dec 5, 2017

Choose a reason for hiding this comment

cdrage Dec 5, 2017

Choose a reason for hiding this comment

cdrage Dec 5, 2017

Choose a reason for hiding this comment

ashetty1 commented Dec 7, 2017

cdrage commented Dec 12, 2017

ashetty1 commented Dec 13, 2017

kadel commented Dec 13, 2017

Adding tests for `kedge build` #381

Adding tests for `kedge build` #381

ashetty1 commented Oct 23, 2017 •

edited