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

Self test, and CI for PRs #120

Open
vorburger opened this issue Mar 4, 2018 · 11 comments
Open

Self test, and CI for PRs #120

vorburger opened this issue Mar 4, 2018 · 11 comments

Comments

@vorburger
Copy link
Collaborator

It could be neat to have a small self contained "self test" (and a CI with it, e.g. Travis, for all PRs) like:

docker build -t fabric8/s2i-java:latest
s2i build s2i-java-example fabric8/s2i-java fabric8/s2i-java-example

and then ideallly, not 100% sure how to do this best, something like:

docker run -p 8080:8080 vorburger:s2i-java-example &
# TODO how to do simplest possible HTTP GET http://localhost:8080 and fail/pass?
# TODO how to best stop docker again
kill $!

I'm happy to move the https://github.com/vorburger/s2i-java-example repo under https://github.com/fabric8io-images, or even just directly contribute that into this repo into some place like s2i/java/example, if either could be of interest or help to get this done.

@vorburger
Copy link
Collaborator Author

and, more minor but while we are at it might as well, if we do something like this, then I guess it should fish-pepper right as part of it, to avoid mistake in contributions changing images/ but not templates/ (I initially made that mistake locally).

@vorburger
Copy link
Collaborator Author

vorburger commented Mar 4, 2018

and to avoid e.g. "fixing the jboss community image but breaking the RHEL image" (or the other way around..) from happening which @jamesnetherton picked up on e.g. in #114, I guess this really should run both the images/jboss as well as the images/rhel ... but that fails for me:

Sending build context to Docker daemon 82.43 kB
Step 1/28 : FROM jboss/openjdk18-rhel7:1.1-7
Trying to pull repository docker.io/jboss/openjdk18-rhel7 ... 
Trying to pull repository registry.fedoraproject.org/jboss/openjdk18-rhel7 ... 
Trying to pull repository registry.access.redhat.com/jboss/openjdk18-rhel7 ... 
Trying to pull repository docker.io/jboss/openjdk18-rhel7 ... 
repository docker.io/jboss/openjdk18-rhel7 not found: does not exist or no pull access

@rhuss
Copy link
Contributor

rhuss commented Mar 5, 2018

Yes, an example makes much sense. 'happy to add it directly to this repository, to keep it simple. Fancy doing a PR ?

wrt/ pulling from registry.access.redhat.com you probably need access to the Red Hat registry. Haven't tried this since quite some time as it build by the product team. @jamesnetherton Any idea what the requirements are for building the rhel based images ?

vorburger added a commit to vorburger/s2i that referenced this issue Mar 7, 2018
@vorburger
Copy link
Collaborator Author

Fancy doing a PR ?

see #127 for a start and lets discuss any feedback on that? You'll figure that the idea is that running the test.sh performs this "self test" initially described above here. (And as of this moment right now, that will actually fail - because of #113 - great, TDD at it's best! 😈)

wrt/ pulling from registry.access.redhat.com you probably need access to

unless @jamesnetherton can provide some guidance how to do this right upstream, lets just wrap this up at least for the community image, and forget about self testing the rhel based image here, for now? Just because from what little I understand of such matters, I somewhat doubt that there is an easy way (with like a test credential or something?), for a test script of the java/images/rhel/ here in an upstream project like this. Probably not worth it then - but having automated test coverage at least for the community image (i.e. java/images/jboss/) would be a great step already I guess.

rhuss added a commit that referenced this issue Mar 18, 2018
add java example and test.sh (#120)
@vorburger
Copy link
Collaborator Author

#136 contributes a working test.sh, which we could probably now use from some service like CircleCI, or Travis. I see that this project actually has a circle.yml, but that does not seem to cause any builds to run on PRs ... does anyone know why? Is there any preference for CircleCI, or can I go ahead and see if we can get this working on https://travis-ci.org, which I have a little bit of experience with?

@rhuss
Copy link
Contributor

rhuss commented Apr 16, 2018

We are currently quite happy with circle, so not really keen on changing to travis.

The reason why the build is running only on master or on tags is that because the build is only used for pushing images to Docker hub. Obviously, you dont want to push an image for a PR build.

But its easy to add a CI test for PR as well. Let's get test.sh fixed first and then check how to adapt circleci.yml ...

vorburger added a commit to vorburger/s2i that referenced this issue May 1, 2018
@vorburger
Copy link
Collaborator Author

Let's get test.sh fixed first and then check how to adapt circleci.yml ...

@rhuss with #136 merged, we could do this now? Dunno exactly what is needed - more than #149 ?

@vorburger
Copy link
Collaborator Author

vorburger commented Nov 13, 2018

@rhuss thought about this again while working on the Java 11 stuff (#160) ... running the ./test.sh, just improved in #183, on all PRs would be really useful for non-regression ... if you would be able to find the cycles to get that set up on CircleCI, that would be really cool!

@vorburger
Copy link
Collaborator Author

I currently manually run ./test.sh on every change manually locally to make sure I don't break anything, but others of course will not (and I would also prefer if PRs would self-verify, instead you having to trust me).

@vorburger
Copy link
Collaborator Author

vorburger commented Nov 21, 2018

This may be as simple as adding ./test.sh to https://github.com/fabric8io-images/s2i/blob/master/.circleci/config.yml ... the problem, as points out in #149 is to have the s2i CLI itself and fish-pepper available in the Circle environment.

@vorburger
Copy link
Collaborator Author

Fixing #223 req for #149 would wrap this up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants