-
-
Notifications
You must be signed in to change notification settings - Fork 40
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
Enhance Testing With Docker #11
Comments
@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? |
Thank you @opalmer, |
@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). 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. |
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.
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 wouldn't have it any other way :). |
Than i would say: Thank you and give it a try :) |
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. |
Okay, fine. Just ping me once you are ready. Just take your time. |
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:
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. |
Thanks for the update. Looks good.
Maybe we can replace localhost with an env var here and only use localhost if the env var is not set. |
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). |
@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 :( |
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 You can also get Gerrit to trust a header provided by the reverse proxy which often works like this:
|
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 overall build time concerns me :( but there could be too many version defined at the moment. Comments? |
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:
The text was updated successfully, but these errors were encountered: