andygrunwald / go-gerrit

Go client/library for Gerrit Code Review
https://godoc.org/github.com/andygrunwald/go-gerrit
MIT License
94 stars 41 forks source link

Enhance Testing With Docker #11

Open opalmer opened 8 years ago

opalmer commented 8 years ago

Ideally it would be nice to test against a local instance of Gerrit. This should provide several new benefits for the project and its contributors:

As part of this enhancement, the following modifications would be made to the code:

opalmer commented 8 years ago

@andygrunwald I'd be curious to know what your thoughts are on this idea. There's a fair amount of work involved to accomplish this and before changing anything I figured it would be a good idea to open this issue to discuss it.

Overall, I'd like to see an increase in test coverage but I felt like mocking out all the responses could lead to more bugs like those that were found in #7 and #9. Additionally it would also be nice to be able to test API calls that modify data without having to worry about modifying 'live' data.

If you're curious how this look from the docker side, I started to take a pass at the setup in this branch:

https://github.com/andygrunwald/go-gerrit/tree/docker_tests

Thoughts?

andygrunwald commented 8 years ago

Thank you @opalmer, i will have a detailed look at the weekend. Ok?

andygrunwald commented 8 years ago

@opalmer Sorry for the long outstanding feedback. Had some private issues that need to be solved.

In general i really like the idea. I think it is a good thing to go into this direction, because with this setup we are able to find, test and fix issues much faster (if a bug happen with a specific version). What i asked myself: This would be more integration tests than unit tests (as we have right now). Isn`t it?

I didn`t tested the setup yet, but it looks promising. A chapter in the readme should be important. With this we enable others to start this setup und contribute as well.

Is it possible to start such a docker setup in travis ci tests? I never saw / tested this. I think those travis tests that run on every commit / pr are important.

Thank you for this good work. I think we should go into this direction.

opalmer commented 8 years ago

This would be more integration tests than unit tests (as we have right now). Isn`t it?

Generally speaking, yes. I think generally that the unittests probably won't go aware entirely and this form of testing should enhance what's already there.

A chapter in the readme should be important. 100% agree here. Docker itself is not difficult to setup or operate but something to connect all the dots would be very beneficial to contributors. There's nothing in the README yet because the general setup is still a work in progress.

Is it possible to start such a docker setup in travis ci tests? I never saw / tested this.

It should be but the current setup will likely require some modification. Travis now runs builds inside of docker and running docker in docker doesn't really work. The good news though is this is pretty much a solved problem already so it should be possible to use docker ourselves: https://docs.travis-ci.com/user/docker/

I think those travis tests that run on every commit / pr are important.

I wouldn't have it any other way :).

andygrunwald commented 8 years ago

Than i would say: Thank you and give it a try :) Do you need something from me? Sadly, my time at the moment is hard limited.

opalmer commented 8 years ago

Nope, I'm good for now thanks. I'll post a PR when there's something to discuss or try out but it will be a while probably before I'm at that point.

andygrunwald commented 8 years ago

Okay, fine. Just ping me once you are ready. Just take your time.

opalmer commented 8 years ago

Just a quick update, I finally got Gerrit running in docker in a way that we'll probably be able to use for testing. Output of setup.sh is below:

> ./setup.sh
Unable to find image 'opalmer/gerrittest:2.13.1' locally
2.13.1: Pulling from opalmer/gerrittest
[ ... ]
Status: Downloaded newer image for opalmer/gerrittest:2.13.1
Waiting up to 120 seconds for Gerrit to come up
+ curl -o /dev/null -s -L 'http://192.168.99.100:8080/login/%23%2F?account_id=1000000' --cookie-jar GerritAccount
+ curl -o /dev/null -s -L http://192.168.99.100:8080/a/accounts/self --digest -u admin:secret
+ set +x
+ ssh-keygen -b 2048 -t rsa -f /var/folders/qp/npkqp_994q79mf6whxs94nbr0000gn/T/tmp.efaPMq29/id_rsa -q -N ''
+ curl -o /dev/null -s -L -X POST http://192.168.99.100:8080/a/accounts/self/sshkeys --digest -u admin:secret -H 'Content-Type: plain/text' --data-binary @/var/folders/qp/npkqp_994q79mf6whxs94nbr0000gn/T/tmp.efaPMq29/id_rsa.pub
+ ssh -o LogLevel=quiet -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -i /var/folders/qp/npkqp_994q79mf6whxs94nbr0000gn/T/tmp.efaPMq29/id_rsa.pub -p 29418 admin@192.168.99.100

So with that digest based auth is setup and allows access to the APIs and we can access ssh too so we can run the gerrit command. There's still some work to be done in order to get the tests using this but it's a start.

andygrunwald commented 8 years ago

Thanks for the update. Looks good.

ADDRESS=localhost

Maybe we can replace localhost with an env var here and only use localhost if the env var is not set.

opalmer commented 7 years ago

So....this is still not done and I haven't had a ton of time to put into the project directly unfortunately.

However, I have developed a library that wraps docker's API in a way that handles the startup/setup/teardown of the docker container that could be reused for this issue: https://github.com/opalmer/dockertest

I've used the above on several projects now so it will probably work for go-gerrit too. We'd still need to make a docker image but that shouldn't be too difficult and is kind of already done in https://github.com/opalmer/gerrittest (which I'm going to eventually rework to use the client library above).

egorse commented 6 years ago

@andygrunwald I am trying the test with docker https://github.com/egorse/go-gerrit/tree/feature/docker-test but, hell, as per https://travis-ci.org/egorse/go-gerrit/builds/353084223 I would guess basic auth is no-go for "older" gerrit version and ldap would be needed :( any advice?

opalmer commented 6 years ago

I had issues getting auth to work with older versions too but I haven't tried in a long time and never got to the bottom of it. While I was working on gerrittest however, which was the thing I wrote to run gerrit in a container/setup the admin account/perform basic operations/etc without depending on go-gerrit, I noticed that it does create a cookie you can use in your requests. You might try pursuing that route since last I checked that's included with every version of Gerrit and is how requests via the UI are authed. Here's there code from gerrittest that you might be able to use or base something off of:

https://github.com/opalmer/gerrittest/blob/master/http.go#L49 https://github.com/opalmer/gerrittest/blob/master/http.go#L255 https://github.com/opalmer/gerrittest/blob/master/cookie.go

You can also get Gerrit to trust a header provided by the reverse proxy which often works like this:

egorse commented 6 years ago

Actually, I resolve authentication by setting up openldap properly a side of gerrit. (Travis build is here - https://travis-ci.org/egorse/go-gerrit/builds/355564183). The gerrit is not populated with fixtures and only test done against real gerrit is checking account for admin.

The overall build time concerns me :( but there could be too many version defined at the moment.

Comments?