Run integration tests on circleci #372

rodrigosaito · 2016-10-06T00:44:47Z

This PR is to address the following issue: #328

This is still working in progress, I'd be happy to finish this if it's looking good.

There are some workarounds to fix the GOPATH in circle.yml, it might not be necessary on the original repo.

peterbourgon · 2016-10-10T07:30:00Z

sd/consul/integration_test.go

@@ -1,4 +1,4 @@
-// +build integration
+// +build integration-ignore


What does this do?

just ignoring those tests for now, will remove that once everything is done.

peterbourgon · 2016-10-10T07:30:43Z

circle.yml

+  pre:
+    - sudo pip install docker-compose
+    - docker-compose -f docker-compose-integration.yml up -d --force-recreate
+    - mkdir -p ${HOME}/.go_workspace/src/github.com/go-kit/ && ln -sf ${HOME}/kit ${HOME}/.go_workspace/src/github.com/go-kit/


Why is this necessary?

it's probably not with go-kit/kit repo, but it's needed with mine. something related to internal packages.

peterbourgon · 2016-10-10T07:31:00Z

circle.yml

+    - docker-compose -f docker-compose-integration.yml up -d --force-recreate
+    - mkdir -p ${HOME}/.go_workspace/src/github.com/go-kit/ && ln -sf ${HOME}/kit ${HOME}/.go_workspace/src/github.com/go-kit/
+  override:
+    - cd ${HOME}/.go_workspace/src/github.com/go-kit/kit && go get -t -d -v ./...


Shouldn't it be sufficient to go get -d -t -v ./... without the cd?

yes, probably with go-kit/kit repo, not with my fork.

before sending the final PR I will remove all this workaround that I need to run builds on circle from my fork.

peterbourgon · 2016-10-19T21:00:26Z

Ping? :)

rodrigosaito · 2016-10-19T21:01:48Z

@peterbourgon sorry about the delay.

so should I keep going with this one?

peterbourgon · 2016-10-19T21:02:39Z

No worries at all! Yes, please, if you can; otherwise I can take it over at some point. Thanks for the contribution :)

rodrigosaito · 2016-10-19T21:04:07Z

Yes, I can and will be happy to help. Will try to finish this by the weekend.

peterbourgon · 2016-11-17T08:16:51Z

Friendly ping :) Any update here?

rodrigosaito · 2016-11-17T14:49:14Z

I am being busy lately, will try to find some time to work on this in the next few days.

rodrigosaito · 2016-11-18T02:20:38Z

@peterbourgon done!

the tests are failing now, probably because circleci is configured to use the old image (12.04) instead of the new one (14.04). If you change that, then the build should pass. Another solution is to stop zookeeper on circle.yml, I can do that if you prefer this solution.

peterbourgon · 2016-11-19T16:35:18Z

Well, the error is pretty explicit; the docker-compose up fails with

ERROR: for zk  Cannot start service zk: failed to create endpoint kit_zk_1 on 
network kit_default: Error starting userland proxy: listen tcp 0.0.0.0:2181: 
bind: address already in use

When I try to do the same thing locally, I get a similar error:

ERROR: for etcd  Cannot start service etcd: driver failed programming 
external connectivity on endpoint kit_etcd_1 (0577dd6...): Error starting 
userland proxy: listen tcp 0.0.0.0:2379: bind: address already in use

I think there must be some error in the way you've got the docker-compose file orchestrating things. Does this work for you locally?

rodrigosaito · 2016-11-19T21:41:19Z

The error happens on circle ci, probably because it's running on 12.04 image which has the zookeeper up and running(https://circleci.com/docs/build-image-precise/#databases), conflicting the ports as the docker-compose is using default ports for every key store (consul, etcd and zk). I tested on circle ci using 14.04 (https://circleci.com/docs/build-image-trusty/#databases) which doesn't have a zookeeper running.

On your machine you might have a etcd running which will also conflict the ports.

Yes it works locally for me and when I run on circle ci from my repo.

I can change to use random ports if you prefer.

peterbourgon · 2016-11-19T21:43:28Z

Ah, OK! Thanks for the more lengthy explanation. You're probably right; I was running that in a minikube (Kubernetes) VM which probably has etcd listening on the default port. I'll poke around a bit more.

rodrigosaito · 2016-12-15T01:41:11Z

hey @peterbourgon any update on this? do you need some help?

peterbourgon · 2016-12-15T11:54:40Z

Thanks for the poke :) I got it working locally. A few changes I'd like to see to the PR itself incoming...

peterbourgon · 2016-12-15T11:56:13Z

circle.yml

+        environment:
+          ETCD_ADDR: http://localhost:2379
+          CONSUL_ADDR: localhost:8500
+          ZK_ADDR: localhost:2181


Can you make whatever changes are necessary to get this working on Circle in the file, or do I need to change something in the repo settings?

I would say that it's better if you go to Circle repo settings and change the box image option from ubuntu 12.04 to 14.04.

Done. Takes a fresh commit to test; we'll do it on master! ;)

peterbourgon · 2016-12-15T11:56:50Z

sd/zk/integration_test.go

-	if err != nil {
-		fmt.Printf("ZooKeeper server error: %v\n", err)
-		os.Exit(1)
+func init() {


Why was this moved out of TestMain and into init?

removed the code that was starting zookeeper (this previously needed the zk jar file somewhere). Not really sure the difference of init and TestMain. Would you prefer to have this code on TestMain instead?

init is executed when the package is imported; TestMain is executed when the test driver starts. I like to avoid init whenever possible, so yes, please :)

ok, got it. Will do that later today.

peterbourgon · 2016-12-15T23:01:26Z

Brilliant! Works locally and I'm happy with the delta. Thanks very much for the contribution! 👍

Run integration tests on circleci

peterbourgon reviewed Oct 10, 2016

View reviewed changes

rodrigosaito force-pushed the integration_tests branch 3 times, most recently from 1a92b06 to 306c92e Compare November 18, 2016 02:17

rodrigosaito changed the title ~~[WIP] Run integration tests on circleci~~ Run integration tests on circleci Nov 18, 2016

peterbourgon reviewed Dec 15, 2016

View reviewed changes

Run integration tests on circleci

0ff31a2

rodrigosaito force-pushed the integration_tests branch from 306c92e to 0ff31a2 Compare December 15, 2016 22:48

peterbourgon merged commit c3ae78b into go-kit:master Dec 15, 2016

rodrigosaito deleted the integration_tests branch December 15, 2016 23:01

jamesgist pushed a commit to jamesgist/kit that referenced this pull request Nov 1, 2024

Merge pull request go-kit#372 from rodrigosaito/integration_tests

630ba09

Run integration tests on circleci

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run integration tests on circleci #372

Run integration tests on circleci #372

rodrigosaito commented Oct 6, 2016

peterbourgon Oct 10, 2016

rodrigosaito Oct 10, 2016

peterbourgon Oct 10, 2016

rodrigosaito Oct 10, 2016

peterbourgon Oct 10, 2016

rodrigosaito Oct 10, 2016

peterbourgon commented Oct 19, 2016

rodrigosaito commented Oct 19, 2016

peterbourgon commented Oct 19, 2016 •

edited

Loading

rodrigosaito commented Oct 19, 2016

peterbourgon commented Nov 17, 2016

rodrigosaito commented Nov 17, 2016

rodrigosaito commented Nov 18, 2016

peterbourgon commented Nov 19, 2016 •

edited

Loading

rodrigosaito commented Nov 19, 2016

peterbourgon commented Nov 19, 2016

rodrigosaito commented Dec 15, 2016

peterbourgon commented Dec 15, 2016

peterbourgon Dec 15, 2016

rodrigosaito Dec 15, 2016

peterbourgon Dec 15, 2016

peterbourgon Dec 15, 2016

rodrigosaito Dec 15, 2016

peterbourgon Dec 15, 2016

rodrigosaito Dec 15, 2016

rodrigosaito Dec 15, 2016

peterbourgon commented Dec 15, 2016

		@@ -1,4 +1,4 @@
		// +build integration
		// +build integration-ignore

Run integration tests on circleci #372

Run integration tests on circleci #372

Conversation

rodrigosaito commented Oct 6, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterbourgon commented Oct 19, 2016

rodrigosaito commented Oct 19, 2016

peterbourgon commented Oct 19, 2016 • edited Loading

rodrigosaito commented Oct 19, 2016

peterbourgon commented Nov 17, 2016

rodrigosaito commented Nov 17, 2016

rodrigosaito commented Nov 18, 2016

peterbourgon commented Nov 19, 2016 • edited Loading

rodrigosaito commented Nov 19, 2016

peterbourgon commented Nov 19, 2016

rodrigosaito commented Dec 15, 2016

peterbourgon commented Dec 15, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterbourgon commented Dec 15, 2016

peterbourgon commented Oct 19, 2016 •

edited

Loading

peterbourgon commented Nov 19, 2016 •

edited

Loading