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

Template validation as part of pull requests #38

Open
jouvin opened this issue Apr 10, 2014 · 23 comments
Open

Template validation as part of pull requests #38

jouvin opened this issue Apr 10, 2014 · 23 comments
Assignees

Comments

@jouvin
Copy link
Contributor

jouvin commented Apr 10, 2014

As discussed during the workshop, it'd be nice to have in all template-library-xxx repositories a validation test associated with pull requests. The initial implementation of the test could be the execution of create-vanilla-SCDB.sh (in SCDB repo). This script pulls down everything needed to compile the examples and compile them. We may just have to check that the script properly report a failure status.

@stdweird
Copy link
Member

@JensTimmerman we need to take a look at this

@jouvin
Copy link
Contributor Author

jouvin commented Jun 24, 2014

With the last version of create-vanilla-SCDB.sh (and get-template-library, release#37), it should be much easier to use it as the test associated with the template-library-xxx repos. We just need to add an option passing the commit to "merge" before the test. I can help with this if you tell me what are the parameters passed by GitHub to the script when calling it as part of a PR.

@jrha jrha added this to the 14.8 milestone Jun 26, 2014
@jrha
Copy link
Member

jrha commented Jun 26, 2014

Lets see if we can get this done for 14.8

@stdweird
Copy link
Member

there are 2 ways forward imho:

  • the prbuilder jenkins test makes a local checkout of the PR branch, the SCDB-vanilla script should use this path instead of checking this repo out. so the vanilla script needs an option that for certain repo it should a path directly instead of doing it's own checkout
  • iin the jenkins environment, you can get the PR branch name, so for this git repo is should checkout this branch instead of the master head. ie vanila script needs option to checkout a certain branch for certain repo.

following relevant GIT_ variables are available during the jenkins job

GIT_BRANCH=origin/pr/34/merge
GIT_URL=git://github.com/quattor/template-library-core.git

and the current checkout is in WORKSPACE

(even better, the vanilla script has a -jenkins option and just figures it all out using the above variables)

@jouvin
Copy link
Contributor Author

jouvin commented Jun 26, 2014

Thanks. I'm trying to come up with an implementation soon... (but probably not today).

@jouvin
Copy link
Contributor Author

jouvin commented Jul 4, 2014

After merging quattor/release#44, you should be able to use create-vanilla-SCDB.sh to implement the pull request testing in the template-library-xxx repos (I advise starting by any repo except this one which is a bit special!).

A typical calling sequence for adding a pull request to the template library before compilinig is:

 create-vanilla-SCDB.sh -F --pull-request template-library-grid:almumontiel:apel:umd-3 HEAD

This will merge the pull request corresponding to branch apel of repository template-library-grid of user almumontiel into branch umd-3 of repository quattor/template-library-grid clone produced by the script before doing the compilation.

Note that the create-vanilla-SCDB.sh script uses get-template-library internally but will take care of downloading it from GitHub.

Let me know if the way to specify the pull request is usable in the GitHub/Jenkins context or if you need a different way of passing the information. The important thing is to be able to pass to the script the source repo/branch to use, even though it is already checked out by GitHub because we cannot just copy the branch content to the standard repo clones as we'll downgrade some files. We really need to ask Git to do a real merge.

@JensTimmerman
Copy link

When a pull request is created in template-library-grid jenkins will check out the template-library-grid master branch, merge in the changes of the pull request, and then run the scripts you want.

If I get your workflow correctly you want it to check out the SCDB master branch, and then use your script to merge in the state of the pull request?
This seems the wrong way around, since you already have the correct merge for the pull request checkout out, you just want to run your compilation against the current checked out directory.

So if I get this correctly the only thing you need to do in the template-library-xxx pull request builder is check out the SCDB master branch, and run a script that takes the current directory instead of checking something out, if this is possible?

@jouvin
Copy link
Contributor Author

jouvin commented Jul 7, 2014

The problem is that we cannot do anything useful (in term of testing) with any single template-library-xxx repos. We need them all (like Pokemons!). This is why the pre-merge done by Jenkins is not really useful in our case.
IMO, the correct workflow is:

  • Get create-vanilla-SCDB.sh available in the Jenkins context. The most flexible is to clone SCDB repo and execute utils/scdb/create-vanilla-SCDB.sh but if this is easier, you can just copy this script from the repo. This script doesn't change very often (now!) as most of the real work happens in get-template-library that it will run.
  • Execute create-vanilla-SCDB.sh with the appropriate arguments, as shown in a previous comment. You need to create the value of the --pull-request based on the information passed by GitHub to Jenkins. I hope the format of the value makes it easy but if it is not the case, let me know: we can adapt the value format to make this use case easy as the option was added for it!
  • Check create-vanilla-SCDB.sh status: it should return a non zero status in case of error.

@JensTimmerman
Copy link

Ok, this makes sense, can you ping me back here after quattor/release#44 is merged, I will look into it.

@jouvin
Copy link
Contributor Author

jouvin commented Jul 7, 2014

I just merge it to allow to proceed with this issue. Be sure to take the last revision of SCDB trunk as I also just merged the create-vanilla-SCDB.sh required.

@jrha
Copy link
Member

jrha commented Aug 19, 2014

@JensTimmerman I'll move the target for this to 14.10 as it doesn't seem like it'll happen by next week.

@jrha jrha modified the milestones: 14.8, 14.10 Aug 19, 2014
@JensTimmerman
Copy link

@jouvin
I've looked at your script but couldn't really grasp it yet, can you point me to the code of get-template-library?
You mention you need all the template-library-xxx branches, but I can't find the code that's checking them out.

So as I understand it now:
we would make a test for each template-library-xxx repository that:

  • check out latest master of all other template-library-xxx repositories (from github)
  • merge the branch set in $GIT_BRANCH (so for the current pull request, set to something like GIT_BRANCH=origin/pr/34/merge)
  • run ant

But it is unclear to me if step 1 is done by the get-template-library script or not?

@jrha jrha modified the milestones: 14.12, 14.10 Nov 5, 2014
@jrha
Copy link
Member

jrha commented Nov 5, 2014

Not a blocker, bumped.

@jouvin
Copy link
Contributor Author

jouvin commented Nov 27, 2014

@JensTimmerman sorry for overlooking your last comment. get-template-library is now part of the release repo but you don't need it. You just need to clone the SCDB repo and execute utils/scdb/create-vanilla-SCDB.sh from it. It will take care of everything needed, including downloading get-template-library and all the appropriate branches from all the repos in the appropriate way.

Typical options for create-vanilla-SCDB.sh are (this is for currently open PR quattor/template-library-grid#113):

utils/scdb/create-vanilla-SCDB.sh -F --pull-request template-library-grid:jouvin:umd3-glue2:umd-3 HEAD

The only thing that needs to be customized at each executiong is the --pull-request value. --help says what is it. Basically the name of the repo, the user who created the PR (the source branch of the PR), the source branch name and the target branch name.

@jrha jrha modified the milestones: 15.6, 14.12 Jan 8, 2015
@stdweird
Copy link
Member

stdweird commented Mar 9, 2015

@jouvin i'm testing latest scdb master, i get

jenkins@jenkins1:~/workspace/test_scdb_build_all/scdb/utils/scdb$ ./create-vanilla-SCDB.sh -F HEAD
Creating vanilla SCDB from ./../.. in /tmp/scdb-vanilla...
Adding ant version apache-ant-1.9.4...
Adding panc version panc-10.1...
Adding scdb-ant-utils version scdb-ant-utils-9.0.2...
Adding svnkit version svnkit-1.8.6...
Downloading template libary (version: HEAD)...
/tmp/quattor-template-library/release/src/scripts/get-template-library: 256: /tmp/quattor-template-library/release/src/scripts/get-template-library: Bad substitution
Failed to download template libary. Check your options: must be valid for this script or get-template-library
usage:  create-vanilla-SCDB.sh [-F] [--debug] [-d scdb_dir] [options...] [quattor_version]

    Valid options are the following ones plus all valid get-template-library options:

        -d scdb_dir : directory where to create SCDB.
                      (D: /tmp/scdb-vanilla)
        --debug : debug mode. Checkout rather than export templates
        -F : remove scdb_dir if it already exists.

it would also be nice if the script would use

git_clone_root=${TMPDIR:-/tmp}/quattor-template-library
scdb_dir=${TMPDIR:-/tmp}/scdb-vanilla

so i can change the /tmp location on jenkins

@jouvin
Copy link
Contributor Author

jouvin commented Mar 15, 2015

The error you report is weird... Are you sure that you use the last version of create-vanilla-SCDB.sh and that you don't have a version of get-template-library located in the same directory as create-vanilla-SCDB.sh (this will prevent retrieving the last version from GitHub).

BTW, I spent some time today fixing --pull-request and it should now really work as documented... in the help of get-template-library or in the upcoming new version of http://www.quattor.org/documentation/2014/06/06/how-to-use-template-library.html (see quattor/quattor.github.com#131 in the meantime).

I'm adding the modification you asked for to be able to relocate /tmp.

@stdweird
Copy link
Member

@jouvin it was a clone in a new directory. i'll look into it again this week

@jouvin
Copy link
Contributor Author

jouvin commented Mar 15, 2015

@stdweird Looking again at the error (I missed the long line...), I think this is caused by a bug I fixed during the last day of the workshop, where the shell was sh which doesn't necessarily translate to bash... See quattor/scdb#32.

@stdweird
Copy link
Member

@jouvin ah yes, you're right

jenkins@jenkins1:~$ ls -l /bin/sh
lrwxrwxrwx 1 root root 4 Nov 13  2013 /bin/sh -> dash

@jouvin
Copy link
Contributor Author

jouvin commented Mar 15, 2015

Download the last version of create-vanilla-SCDB.

@jouvin
Copy link
Contributor Author

jouvin commented Mar 19, 2015

@stdweird let me know if there is anything I could do to help getting this working...

@jouvin
Copy link
Contributor Author

jouvin commented Apr 18, 2015

It'd be good to have it working before 15.4...

@jouvin jouvin modified the milestones: 15.6, 15.8 May 14, 2015
@jouvin
Copy link
Contributor Author

jouvin commented May 14, 2015

@stdweird what can we do to complete this...??? Issues found during RC2 would have been avoided if running the suggested unit tests... I'm reassigning to 15.6 with the hope that we can get it working before that.

@jrha jrha modified the milestones: 15.8, 15.6 Jun 2, 2015
@jrha jrha modified the milestones: 16.4, 15.12 Dec 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants