On Monday 12 November 2012 05:40 PM, Senthil Kumaran S wrote:

On Monday 12 November 2012 04:18 PM, Senthil Kumaran S wrote:

...

We now have support for pulling git repositories which holds the test definition, by defining it in the job file. The code is available here - https://code.launchpad.net/~stylesen/lava-dispatcher/testdef-from-repo

Will shortly push support for bzr.

I just pushed in support for pulling from bzr branches. The last thing pending is, support for pulling specific revisions.

Forgot the job definition I used in order to test this new stuff. Here it is attached.

Thank You.

On 11/12/2012 06:52 AM, Senthil Kumaran S wrote:

Hi,

On Monday 12 November 2012 05:40 PM, Senthil Kumaran S wrote:

...

I just pushed in support for pulling from bzr branches. The last thing pending is, support for pulling specific revisions.

Support for pulling specific revisions from the repository which holds test definitions is pushed in! Use the job file attached in this email to check this feature.

I just spent a little time looking at the branch and have a few comments:

= the action changes:

We now have:

"command": "lava_test_shell", "parameters": { "testdef_repos": [ {"repo1" : "git://git.linaro.org/people/stylesen/sampletestdefs.git", "testdef1": {"test": "testdef.yaml", "files": ["testsample.sh", "tester.sh"]}, "testdef2": {"test": "testdef.yaml", "files": ["testsample.sh", "tester.sh"]} }, {"repo2" : "git://git.linaro.org/people/stylesen/sampletestdefs.git", "testdef": {"test": "testdef.yaml", "files": ["testsample.sh", "tester.sh"]} }], "timeout": 1800 }

There are 2 things I don't like. The "testdefX" thing and the "files". I see where you are coming from on the "files" piece, but I think we should just pull in the whole repository for the test. I see how you are trying to eliminate copying the same stuff multiple times. However, we can probably be smart and have a directory on the target that has a single copy of each repo and then do symlinks to it for each test. This would then allow us to do a definition like:

"command": "lava_test_shell", "parameters": { "testdef_repos": [ { "git-repo" : "foo.git", "testdefs": [ "testdef1.yaml", "t2.yaml" ] ] }], "timeout": 1800 }

= _get_test_def_repo: as noted in my proposal, lets not try and guess between git/bzr. The code we have here misses things like cloning a repo using HTTP which would be hard to guess about.

= dict.get(key) logic: Most of the dispatcher uses dict[key]. Lets change to use that to keep things consistent.

= _configure_target: The testdef_repos and testdef_urls snippets are making this function grow pretty large. Let's break those out into their own functions.

Andy Doan andy.doan@linaro.org writes:

On 11/12/2012 06:52 AM, Senthil Kumaran S wrote:

...

Hi,

On Monday 12 November 2012 05:40 PM, Senthil Kumaran S wrote:

...

I just pushed in support for pulling from bzr branches. The last thing pending is, support for pulling specific revisions.

Support for pulling specific revisions from the repository which holds test definitions is pushed in! Use the job file attached in this email to check this feature.

I just spent a little time looking at the branch and have a few comments:

= the action changes:

We now have:

   "command": "lava_test_shell",
   "parameters": {
       "testdef_repos": [
         {"repo1" : 

"git://git.linaro.org/people/stylesen/sampletestdefs.git", "testdef1": {"test": "testdef.yaml", "files": ["testsample.sh", "tester.sh"]}, "testdef2": {"test": "testdef.yaml", "files": ["testsample.sh", "tester.sh"]} }, {"repo2" : "git://git.linaro.org/people/stylesen/sampletestdefs.git", "testdef": {"test": "testdef.yaml", "files": ["testsample.sh", "tester.sh"]} }], "timeout": 1800 }

There are 2 things I don't like. The "testdefX" thing and the "files". I see where you are coming from on the "files" piece, but I think we should just pull in the whole repository for the test. I see how you are trying to eliminate copying the same stuff multiple times. However, we can probably be smart and have a directory on the target that has a single copy of each repo and then do symlinks to it for each test.

Or just not bother trying to be smart (I expect the large majority of lava_test_shell actions that we execute to specify only one test).

I had always envisioned that we would just clone the testdef containing repository onto the board. Basically I think we should aim for the simplest case: that, for example, the PMWG just need to drop a testdef.yaml file into http://git.linaro.org/gitweb?p=tools/pm-qa.git%3Ba=tree to and put git://git.linaro.org/tools/pm-qa.git into the job json in the appropriate place. This means that we should just copy all files in the branch across and that we should have a default location for the testdef YAML. Sorry if I missed a chance to make this clear earlier!

This would then allow us to do a definition like:

   "command": "lava_test_shell",
   "parameters": {
       "testdef_repos": [
         {
           "git-repo" : "foo.git",
           "testdefs": [ "testdef1.yaml", "t2.yaml" ]
           ]
         }],
       "timeout": 1800
   }

= _get_test_def_repo: as noted in my proposal, lets not try and guess between git/bzr. The code we have here misses things like cloning a repo using HTTP which would be hard to guess about.

+1

= dict.get(key) logic: Most of the dispatcher uses dict[key]. Lets change to use that to keep things consistent.

= _configure_target: The testdef_repos and testdef_urls snippets are making this function grow pretty large. Let's break those out into their own functions.

Hi Andy,

On Monday 12 November 2012 11:41 PM, Andy Doan wrote:

I just spent a little time looking at the branch and have a few comments:

I just made sure all your comments are addressed in revision 448. Please have a look at it. The new job file which I used to test this is attached.

<snip> The following changes are incorporated as per Andy's review comments:

1) Remove testdefX 2) Remove specifying 'files' 3) Do not guess git/bzr repos 4) Get rid of .get() syntax for accessing dictionary keys whereever possible 5) Split code in _configure_target 6) Copy all the files required for test to target directory (mwhudson) - no symlinks </snip>

Thank You.

Hi Andy,

I pushed some changes @ revision 449 which fixes the following:

On Thursday 15 November 2012 11:15 PM, Andy Doan wrote:

This is starting to look pretty good. I'll make a couple of nit-picks:

1. has_key

You have a couple of things like:

if testdef_repo.has_key('bzr-repo')

I think has_key is being deprecated and its more accepted to instead do:

This is done.

2. the counter in _get_test_definition_from_repo and

_get_test_definition_from_url

Then the functions can just return the array of directories and not the tuple.

Used a timestamp value for this instead of a counter.

3. _copy_test (not sure about this comment)

This function has logic to copy the repo to the target with shutil.copy2. I wonder if this is needed. When you look at the logic of _configure_target, the repo cloning logic will be called while the target's filesystem is mounted. So could we just do the bzr/git clone straight to that directory and then avoid the copy operation later?

A bit confused on this. Unsure on how to achieve this :(

Thank You.

On Friday 16 November 2012 09:05 PM, Andy Doan wrote:

I think its time to submit this branch as a merge proposal.

I just created a merge proposal.

Thank You.

Senthil Kumaran S senthil.kumaran@linaro.org writes:

Hi Andy,

On Monday 12 November 2012 11:41 PM, Andy Doan wrote:

...

I just spent a little time looking at the branch and have a few comments:

I just made sure all your comments are addressed in revision 448. Please have a look at it. The new job file which I used to test this is attached.

Hi, I think this is looking pretty good. After you've addressed Andy's comments please propose the branch for review!

Cheers, mwh