Merge lp:~mrazik/jenkins-launchpad-plugin/stale-locks-take2 into lp:~private-ps-quality-team/jenkins-launchpad-plugin/trunk

[Martin Mrazik]

This removes the locking mechanism and instead queries jenkins if a specific job is running (with specific params).

Take 2 on removing locks and querying jenkins instead.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:133
http://jenkins.qa.ubuntu.com/job/jenkins-launchpad-plugin-ci/7/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/jenkins-launchpad-plugin-saucy-amd64-ci/8/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/jenkins-launchpad-plugin-ci/7/rebuild

review: Needs Fixing (continuous-integration)

[lp:~mrazik/jenkins-launchpad-plugin/stale-locks-take2]

134. By mrazik on 2013-05-31

pep8 fixed

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:134
http://jenkins.qa.ubuntu.com/job/jenkins-launchpad-plugin-ci/8/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/jenkins-launchpad-plugin-saucy-amd64-ci/9

Click here to trigger a rebuild:
http://s-jenkins:8080/job/jenkins-launchpad-plugin-ci/8/rebuild

review: Approve (continuous-integration)

[Martin Mrazik (mrazik)]

I accidentally removed test_socketlock and then added it back. There were not changes in that file.

[Martin Mrazik (mrazik)]

One thing that concerns me a bit is how much load this checking will generate. I'll probably need to do some testing first on a real jenkins (by changing the triggering job to use this branch or something the like).

[Francis Ginther (fginther)]

I still need to review the tests, but I have the following comments (as discussed in our 1 on 1):

In trigger_ci_build() and trigger_al_build(), the
    if launchpadutils.testing_in_progress(mp, jenkins_job):
        continue

check should be the last check before proceeding to the trigger as it is likely the most time and resource intensive check.

review: Needs Fixing

[lp:~mrazik/jenkins-launchpad-plugin/stale-locks-take2]

135. By mrazik on 2013-06-04

moving testing_in_progress check to the very end of trigger_al_build/trigger_ci_build

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:135
http://jenkins.qa.ubuntu.com/job/jenkins-launchpad-plugin-ci/10/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/jenkins-launchpad-plugin-saucy-amd64-ci/11/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/jenkins-launchpad-plugin-ci/10/rebuild

review: Needs Fixing (continuous-integration)

[lp:~mrazik/jenkins-launchpad-plugin/stale-locks-take2]

136. By mrazik on 2013-06-04

fixed tests

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:136
http://jenkins.qa.ubuntu.com/job/jenkins-launchpad-plugin-ci/11/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/jenkins-launchpad-plugin-saucy-amd64-ci/12

Click here to trigger a rebuild:
http://s-jenkins:8080/job/jenkins-launchpad-plugin-ci/11/rebuild

review: Approve (continuous-integration)

[Martin Mrazik (mrazik)]

the triggering job is now using this new branch. So far I didn't see anything strange and the build time of trigger-ci-and-autolanding-job-NG is roughly the same as before.

There is one issue, though -- fasttrack merges. I just noticed a pointless merge and as far as I can see it is because the autolanding was triggered while the other generic-land (which I started manually due to some unrelated bug) was still running.
OTOH the problem is probably not introduced by this branch and explains why I'm seeing these pointless merges every now and then.

[Martin Mrazik (mrazik)]

Just for the reference. This was the first trigger-ci-and-autolanding-job-NG build configured with this branch:
  #8425 Jun 4, 2013 6:17:18 AM

[Martin Mrazik (mrazik)]

> There is one issue, though -- fasttrack merges. I just noticed a pointless
> merge and as far as I can see it is because the autolanding was triggered
> while the other generic-land (which I started manually due to some unrelated
> bug) was still running.
> OTOH the problem is probably not introduced by this branch and explains why
> I'm seeing these pointless merges every now and then.

I was wrong about this. It is a newly introduced regression. OTOH solving this would most likely add yet another level of complexity and:
1. even if it happens nothing really bad happens (just the pointless merge error)
2. it shouldn't be happening very often as the generic-land is usually finished in 15 mins (the time between to triggering operations)

[Francis Ginther (fginther)]

> > There is one issue, though -- fasttrack merges. I just noticed a pointless
> > merge and as far as I can see it is because the autolanding was triggered
> > while the other generic-land (which I started manually due to some unrelated
> > bug) was still running.
> > OTOH the problem is probably not introduced by this branch and explains why
> > I'm seeing these pointless merges every now and then.
>
> I was wrong about this. It is a newly introduced regression. OTOH solving this
> would most likely add yet another level of complexity and:
> 1. even if it happens nothing really bad happens (just the pointless merge
> error)
> 2. it shouldn't be happening very often as the generic-land is usually
> finished in 15 mins (the time between to triggering operations)

Right. The fasttrack MPs never call the autolanding job, they go straight to generic-land. As this new method looks for the MPs that were started from the autolanding job, it won't find a downstream build due to the upstream check.

I agree that we are safe for now as long as the second generic-land does not change the state of the MP from Merged to something else.

[Francis Ginther (fginther)]

I think a testcase is needed with a different MP in a downstream job in TestIsJobOrDownstreamBuildingScenarios:
        ('downstream_job_building_with_different_mp',
         {'expected': False,
          'job_params': {'merge_proposal': 'http://mp'},
          'jenkins_data': {
              'builds': [{
                  'building': True,
                  'actions': [{
                      'causes': [{
                          'upstreamProject': 'job',
                          'upstreamBuild': 22
                      }]
                  }],
              'result': None}],
              'upstreamProjects': [{
                  'name': 'job',
                  'builds': [
                      {
                          'number': 22,
                          'actions': [{'parameters': [{
                              'name': 'merge_proposal',
                              'value': 'http://mp3'}]}]}
                  ]}]}}),

The scenario is that job was started with "http://mp3" and it is still active in a downstream build (like generic-land). A new MP, "http://mp" is being checked, no matching builds should be found. I'm guessing at the correct scenario, it may not be quite right.

Some comments appear to be out of place:
- In is_job_or_downstream_building(), the docstring refers to "the json request below" which is actually in is_downstream_job_running().
- is_job_or_downstream_building() and _is_item_queued_by_upstream() are missing parameters and return value in the docstring.
- In _get_build_actions(), there is a comment, "# get the jenkins queue" which does not belong.

Other then that, this looks good.

review: Needs Fixing

[lp:~mrazik/jenkins-launchpad-plugin/stale-locks-take2]

137. By mrazik on 2013-06-05

added a downstream_job_building_with_different_mp scenario for is_job_or_downstream_building test

138. By mrazik on 2013-06-05

fixed some docstrings

[Martin Mrazik (mrazik)]

> I think a testcase is needed with a different MP in a downstream job in
> TestIsJobOrDownstreamBuildingScenarios:
> ('downstream_job_building_with_different_mp',
> {'expected': False,
> 'job_params': {'merge_proposal': 'http://mp'},
> 'jenkins_data': {
> 'builds': [{
> 'building': True,
> 'actions': [{
> 'causes': [{
> 'upstreamProject': 'job',
> 'upstreamBuild': 22
> }]
> }],
> 'result': None}],
> 'upstreamProjects': [{
> 'name': 'job',
> 'builds': [
> {
> 'number': 22,
> 'actions': [{'parameters': [{
> 'name': 'merge_proposal',
> 'value': 'http://mp3'}]}]}
> ]}]}}),
>
> The scenario is that job was started with "http://mp3" and it is still active
> in a downstream build (like generic-land). A new MP, "http://mp" is being
> checked, no matching builds should be found. I'm guessing at the correct
> scenario, it may not be quite right.

Added.

> Some comments appear to be out of place:
> - In is_job_or_downstream_building(), the docstring refers to "the json
> request below" which is actually in is_downstream_job_running().
> - is_job_or_downstream_building() and _is_item_queued_by_upstream() are
> missing parameters and return value in the docstring.
> - In _get_build_actions(), there is a comment, "# get the jenkins queue" which
> does not belong.

Fixed.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:138
http://jenkins.qa.ubuntu.com/job/jenkins-launchpad-plugin-ci/14/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/jenkins-launchpad-plugin-saucy-amd64-ci/15

Click here to trigger a rebuild:
http://s-jenkins:8080/job/jenkins-launchpad-plugin-ci/14/rebuild

review: Approve (continuous-integration)

[Francis Ginther (fginther)]

Thanks for the updates, approve.

review: Approve

=== modified file 'jlp.config'
--- jlp.config	2013-05-16 08:20:42 +0000
+++ jlp.config	2013-06-05 09:44:54 +0000
@@ -40,9 +40,6 @@
 #Usually you don't need to change this
 launchpad_review_type: continuous-integration
 
-# directory containing lockfiles for Launchpad merge proposals
-launchpadlocks_dir: /tmp/jenkins-launchpad-plugin/locks
-
 #lock file that is being used to limit the number of parallel launchpad
 #connections
 lock_name: launchpad-trigger-lock
 
=== modified file 'jlp/__init__.py'
--- jlp/__init__.py	2013-05-16 08:23:40 +0000
+++ jlp/__init__.py	2013-06-05 09:44:54 +0000
@@ -89,8 +89,6 @@
 assert Branch  # silence pyflakes
 from .bzrrecipe import BzrRecipe
 assert BzrRecipe  # silence pyflakes
-from .mergeproposalreview import MergeProposalReview
-assert MergeProposalReview  # silence pyflakes
 from .launchpadagent import get_launchpad
 assert get_launchpad  # silence pyflakes
 from .dputrunner import DputRunner
 
=== modified file 'jlp/commands/autoland.py'
--- jlp/commands/autoland.py	2013-05-17 15:53:01 +0000
+++ jlp/commands/autoland.py	2013-06-05 09:44:54 +0000
@@ -2,7 +2,7 @@
 from argparse import ArgumentParser
 import argparse
 from jlp.launchpadutils import build_state, LaunchpadVote
-from jlp import (launchpadutils, MergeProposalReview, Branch,
+from jlp import (launchpadutils, Branch,
                  DputRunner, jenkinsutils, get_launchpad,
                  get_config_option, logger)
 from tarmac.exceptions import TarmacMergeError, BranchHasConflicts
@@ -156,8 +156,6 @@
         return 1
     finally:
         target.cleanup()
-        review = MergeProposalReview(mp)
-        review.unlock()
 
     return 0
 
 
=== modified file 'jlp/jenkinsutils.py'
--- jlp/jenkinsutils.py	2013-05-20 11:44:39 +0000
+++ jlp/jenkinsutils.py	2013-06-05 09:44:54 +0000
@@ -6,7 +6,6 @@
 from textwrap import dedent
 from . import get_json_jenkins
 from lazr.restfulclient.errors import Unauthorized
-from .mergeproposalreview import MergeProposalReview
 from jlp import logger, get_config_option
 
 
@@ -15,6 +14,315 @@
                             urlparse.urlparse(url).path.replace('//', '/'))
 
 
+def _actions_has_param(actions, param, value):
+    """ Internal method to see if actions json object (as returned by jenkins)
+    includes a given parameter (and value).
+
+    It returns True or False based on whether a parameter/value pair is
+    included in the given jenkins job actions ("actions" is where jenkins
+    stores the parameters)
+
+    :param actions: json result (list of dictionaries) as returned by jenkins
+    :param param: name of the jenkins job parameter to check
+                  (e.g. 'merge_proposal')
+    :param value: value that we want to check
+
+    Returns True/False.
+    """
+    for action in actions:
+        if 'parameters' not in action:
+            continue
+        for parameter in action['parameters']:
+            if parameter['name'] == param and 'value' in parameter and \
+                    parameter['value'] == value:
+                return True
+    return False
+
+
+def _actions_has_all_params(actions, job_params):
+    """ Internal method to see if actions json object (as returned by jenkins)
+    includes all given job_params. See also _actions_has_param.
+
+    The main difference between this and _actions_has_param is that this
+    method checks for all params in a given dictionary (job_params).
+
+    :param actions: json result (list of dictionaries) as returned by jenkins
+    :param job_params: dictionary of parameters to check in the form of:
+                       {'parameter': 'value', 'parameter2': 'value2'}
+
+    Returns True/False.
+    """
+    all_params = True
+    for param in job_params:
+        all_params = all_params and \
+            _actions_has_param(actions, param, job_params[param])
+    return all_params
+
+
+def get_running_builds(job, job_params={}):
+    """ For a given jenkins job return a list of currently active builds
+        :param job: name of the jenkins job
+        :param job_params: dictionary of job parameters
+
+        Returns an empty list if there is no active build with given
+        parameters. If job_params is empty (default) it is assumed no check
+        for job_params is required.
+    """
+
+    json_jenkins = get_json_jenkins()
+    jenkins_url = get_config_option('jenkins_url')
+    request = \
+        '{}/job/{}/api/json'.format(jenkins_url, job) +\
+        '?depth=1&tree=builds[number,building,actions[parameters[name,value]]]'
+    data = json_jenkins.get_json_data(request, append_api=False)
+    if 'builds' not in data:
+        return []
+    result = []
+
+    for build in data['builds']:
+        if build['building'] and \
+                _actions_has_all_params(build['actions'], job_params):
+            result.append(build['number'])
+    return result
+
+
+def _get_build_actions(job, build):
+    """Internal method. For a given job and build return the "actions" object
+    as returned by jenkins. This object holds the list of parameters of a
+    given build.
+
+    :param: job: jenkins job name
+    :param: build: jenkins build number
+    """
+
+    json_jenkins = get_json_jenkins()
+    jenkins_url = get_config_option('jenkins_url')
+
+    job = json_jenkins.get_json_data('{}/job/{}/{}'.format(
+        jenkins_url, job, build))
+    if 'actions' not in job:
+        return []
+    return job['actions']
+
+
+def _is_item_queued_by_upstream(item, upstream_job, upstream_params):
+    """Internal method to check if a given queued item
+    (as returned by jenkins; see the "$jenkins_url/queue/json/api" API) was
+    queued by an upstream job with a given upstream_job parameters.
+
+    The idea here is to check e.g. if generic_land (item) was queued by
+    unity-autolanding (upstream_job) with {'merge_proposal': 'http://'}
+    (upstream_params). If there is such a generic-land queued then this method
+    returns True otherwise it returns False
+
+    :param item: "item" as returned in the json jenkins reply (see the queue
+                 API)
+    :param: upstream_job: name of the upstream_job
+    :param upstream_params: dictionary of the upstream job params we are
+           interested in
+    """
+    upstream_project = None
+    upstream_build = None
+
+    if 'actions' not in item:
+        return False
+
+    for action in item['actions']:
+        upstream_project = None
+        if 'causes' not in action:
+            continue
+        for cause in action['causes']:
+            if isinstance(cause, dict) and 'upstreamProject' in cause:
+                upstream_project = cause['upstreamProject']
+                # if the upstreamProject differes we can return False right
+                # away
+                if upstream_project != upstream_job:
+                    return False
+
+                upstream_build = cause['upstreamBuild']
+                if upstream_params:
+                    upstream_actions = _get_build_actions(
+                        upstream_project, upstream_build)
+                    if _actions_has_all_params(upstream_actions,
+                                               upstream_params):
+                        return True
+                else:
+                    return True
+    return False
+
+
+def is_job_queued(job, queue=None, job_params={}, upstream_job=None,
+                  upstream_params={}):
+    """ Return if a job is queued in a given jenkins queue.
+        If upstream_job is supplied the job in queue must have been triggered
+        by upstream_job.
+        If upstream_params are given then the triggering upstream_job must
+        include the given upstream_params (dictionary of the form of
+        {'param1': 'value1'}).
+
+        :param job: jenkins job name
+        :param queue: jenkins queue (as returned by $jenkinsurl/queue/api/json)
+        :param upstream_job: name of the upstream job that was supposed to
+               queue job
+        :param upstream_params: parameters to check in the upstream_job
+    """
+    if queue is None:
+        json_jenkins = get_json_jenkins()
+        jenkins_url = get_config_option('jenkins_url')
+        queue = json_jenkins.get_json_data('{}/queue'.format(jenkins_url))
+
+    for item in queue['items']:
+        if 'task' in item and 'name' in item['task']:
+            # we can ignore jobs that are different from the job we are
+            # looking for
+            if item['task']['name'] != job:
+                continue
+
+            # check if the required parameters are present
+            if job_params and 'actions' in item and \
+                    not _actions_has_all_params(item['actions'], job_params):
+                continue
+
+            # if we have a match and we don't need to check
+            # for upstream_job then we are done
+            if upstream_job is None:
+                return True
+
+            if _is_item_queued_by_upstream(item,
+                                           upstream_job,
+                                           upstream_params):
+                return True
+
+    return False
+
+
+def is_downstream_job_running(downstream_project,
+                              upstream_job, upstream_params):
+    """Check if downstream job is running.
+    It returns True only if the downstream_project was triggered by
+    upstream_job and upstream_job has upstream_params (e.g. a given merge
+    proposal).
+
+    This is unfortunately not 100% reliable. For some reasons the json request
+    in (&tree=builds[number,url,...]) sometimes doesn't return what it should
+    (maybe the jenkins needs to update some internal state). Once the
+    relationship between jobs is somehow recorded the subsequent calls always
+    return them correctly.
+
+    :param downstream_project: name of the jenkins downstream_project (e.g.
+           generic-land)
+    :param upstream_job: name of the upstream_job (e.g. unity-autolanding)
+    :param upstream_params: dictionary of the parameters (e.g.
+           {'merge_proposal': 'http://...'})
+    """
+    json_jenkins = get_json_jenkins()
+    jenkins_url = get_config_option('jenkins_url')
+    json_request = \
+        '/api/json?depth=2&tree=builds[number,url,building,' + \
+        'actions[causes[upstreamBuild,upstreamProject]]],' + \
+        'upstreamProjects[name,builds[number,actions[parameters[name,value]]]]'
+
+    request = '{}job/{}{}'.format(
+        jenkins_url, downstream_project, json_request)
+
+    data = json_jenkins.get_json_data(request, append_api=False)
+    for build in data['builds']:
+        if 'actions' not in build:
+            continue
+        for action in build['actions']:
+            if 'causes' not in action:
+                continue
+            for cause in action['causes']:
+                upstream_project = None
+                if isinstance(cause, dict) and 'upstreamProject' in cause:
+                    upstream_project = cause['upstreamProject']
+                    upstream_build = cause['upstreamBuild']
+
+            if build['building'] and upstream_project == upstream_job:
+                #now we need to find the builds of the right upstream_job
+                #in data['upstreamProjects']
+                builds = {}
+                for upstream_project in data['upstreamProjects']:
+                    if upstream_project['name'] == upstream_job:
+                        builds = upstream_project['builds']
+
+                #find the upstream build in the data
+                for upstreamBuild in builds:
+                    if upstreamBuild['number'] != upstream_build:
+                        continue
+                    if _actions_has_all_params(upstreamBuild['actions'],
+                                               upstream_params):
+                        return True
+    return False
+
+
+def is_job_or_downstream_building(job, job_params):
+    """Check if a job (with given parameters) or its downstream job is
+    currently building or not.
+
+    Usually this will be called like this:
+        is_job_or_downstream_building(
+            'my-project-ci',
+            {'merge_proposal': 'http://my-merge-proposal/...'})
+    It will return True iff:
+       - my-project-ci build is running or queued and it has the correct
+         merge_proposal param
+       - a downstream project (e.g. generic-land) is running (or queued) and
+         my-project-ci which triggered the downstream has the right
+         merge_proposal param
+
+    This is unfortunately not 100% reliable because is_downstream_job_running
+    is not 100% reliable. See the docstring there for more info.
+
+    :param job: name of the jenkins job
+    :param job_params: parameters of the jenkins job we want to check
+    """
+
+    json_jenkins = get_json_jenkins()
+    jenkins_url = get_config_option('jenkins_url')
+
+    # get the jenkins queue. This is to prevent a case when a job is queued
+    # while we are preforming the other checks. It might be the other way
+    # round (i.e. a job will get out of the queue and finish its execution)
+    # and our result won't be correct. Doing it this way is however safer. The
+    # worst thing that can happen is that our job will be scheduled with the
+    # next poll
+    queue = json_jenkins.get_json_data('{}/queue'.format(jenkins_url))
+
+    # first check if a job with a given name and given params isn't already
+    # running
+    if get_running_builds(job, job_params):
+        logger.debug('{} is currently building'.format(job))
+        return True
+
+    # next check if such job (including the params) is not queued
+    if is_job_queued(job, queue, job_params=job_params):
+        logger.debug('{} is currently queued'.format(job))
+        return True
+
+    # now do similar checks like the above for the (directly) downstream
+    # projects
+    downstream_projects = get_downstream_projects(
+        json_jenkins, '{}/job/{}'.format(jenkins_url, job))
+
+    for downstream_project in downstream_projects:
+        # first check the queue; If the downstream_project is in the queue
+        # and was triggered by job then we are done
+        if is_job_queued(downstream_project, queue, upstream_job=job,
+                         upstream_params=job_params):
+            logger.debug('Downstream project "{}" is queued.'.format(
+                downstream_project))
+            return True
+
+        # build is not queued. Lets check if it is not running
+        if is_downstream_job_running(downstream_project, job, job_params):
+            logger.debug('Downstream project "{}" is building'.format(
+                downstream_project))
+            return True
+
+    return False
+
+
 def is_job_published(job):
     if not job:
         logger.debug('is_job_published: no job specified')
@@ -353,14 +661,11 @@
     else:
         logger.debug('Doing a regular build')
 
-    review = MergeProposalReview(mp)
     try:
         logger.debug('Starting job: ' + str(jenkins_params))
-        review.lock(jenkins_job)
         j.build_job(jenkins_job, jenkins_params)
     except jenkins.JenkinsException:
         logger.debug('Starting a job failed')
-        review.unlock()
         message = "ERROR: failed to start a jenkins job"
         mp.createComment(
             review_type=get_config_option('launchpad_review_type'),
@@ -498,12 +803,12 @@
         if not launchpadutils.user_allowed_to_trigger_jobs(mp.registrant):
             continue
 
-        if launchpadutils.testing_in_progress(mp):
-            continue
-
         if launchpadutils.latest_candidate_validated(launchpad_user, mp):
             continue
 
+        if launchpadutils.testing_in_progress(mp, jenkins_job):
+            continue
+
         ret = start_jenkins_job(launchpad_user, jenkins_url, jenkins_job, mp)
         result = ret and result
     return result
@@ -544,13 +849,13 @@
                 launchpadutils.LaunchpadVote.NEEDS_FIXING)
             continue
 
-        if launchpadutils.testing_in_progress(mp):
-            continue
-
         if launchpadutils.unapproved_prerequisite_exists(mp):
             logger.debug('Unapproved prerequisite exists for ' + str(mp))
             continue
 
+        if launchpadutils.testing_in_progress(mp, jenkins_job):
+            continue
+
         logger.debug('Going to trigger merge: ' + str(mp.source_branch))
 
         launchpad_user = lp_handle.people(
 
=== modified file 'jlp/launchpadutils.py'
--- jlp/launchpadutils.py	2013-05-10 14:10:52 +0000
+++ jlp/launchpadutils.py	2013-06-05 09:44:54 +0000
@@ -1,6 +1,5 @@
 import re
 from jlp import logger, get_config_option
-from .mergeproposalreview import MergeProposalReview
 import jenkinsutils
 from . import get_json_jenkins
 
@@ -42,8 +41,6 @@
         mp.createComment(
             review_type=get_config_option('launchpad_review_type'),
             subject=subject, content=message, vote=vote)
-        review = MergeProposalReview(mp)
-        review.unlock()
     else:
         mp.createComment(
             review_type=get_config_option('launchpad_review_type'),
@@ -172,17 +169,17 @@
     return False
 
 
-def testing_in_progress(mp):
+def testing_in_progress(mp, jenkins_job):
     """ Returns whether testing is in progress.
 
-    We are using a file (represented by MergeProposalReview class) to indicate
-    a testing is in progress. This file is created when the jenkins job is
-    started and cleaned up either by generic-update-mp job or generic-land.
+    This method queries jenkins to find out if we are not running a similar job
+    (or downstream such as generic_land) already.
 
     :param mp: launchpad merge proposal handle
+    :param jenkins_job: name of the jenkins job that is going to be triggered
     """
-    review = MergeProposalReview(mp)
-    if review.is_locked():
+    if jenkinsutils.is_job_or_downstream_building(
+            jenkins_job, job_params={'merge_proposal': mp.web_link}):
         logger.debug('Skipping this MP. It is curently being ' +
                      'tested by Jenkins.')
         return True
@@ -279,8 +276,6 @@
     mp.createComment(review_type=get_config_option('launchpad_review_type'),
                      vote=LaunchpadVote.APPROVE, subject=state,
                      content=content)
-    review = MergeProposalReview(mp)
-    review.unlock()
 
 
 def disapprove_mp(mp, revision, build_url, reason=None):
@@ -305,8 +300,6 @@
     mp.createComment(review_type=get_config_option('launchpad_review_type'),
                      vote=LaunchpadVote.NEEDS_FIXING, subject=subject,
                      content=content)
-    review = MergeProposalReview(mp)
-    review.unlock()
 
 
 class UpdateMergeProposalException(Exception):
 
=== removed file 'jlp/mergeproposalreview.py'
--- jlp/mergeproposalreview.py	2013-05-10 14:10:52 +0000
+++ jlp/mergeproposalreview.py	1970-01-01 00:00:00 +0000
@@ -1,55 +0,0 @@
-import os
-import hashlib
-from jlp import logger, get_config_option
-
-
-class MergeProposalReview():
-    _mp = None
-
-    def __init__(self, mp):
-        self._mp = mp
-
-    @property
-    def mp(self):
-        """Merge proposal."""
-
-        return self._mp
-
-    def _get_hash(self):
-        return hashlib.sha224(self.mp.web_link).hexdigest()
-
-    def get_lockfile(self):
-        return '{dir}/{hash}'.format(
-            dir=get_config_option('launchpadlocks_dir'),
-            hash=self._get_hash())
-
-    def lock(self, job_name):
-        locks_dir = get_config_option('launchpadlocks_dir')
-        if not os.path.exists(locks_dir):
-            os.makedirs(locks_dir)
-        lockfile = open(self.get_lockfile(), 'w')
-        lock_info = "{jenkins_job}\n{merge_proposal}\n"
-        lock_info = lock_info.format(jenkins_job=job_name,
-                                     merge_proposal=self.mp.web_link)
-        lockfile.write(lock_info)
-        lockfile.close()
-
-    def unlock(self):
-        try:
-            os.remove(self.get_lockfile())
-            return True
-        except OSError as error:
-            if error.errno == 2:
-                logger.debug("mp lock doesn't exists. Ignoring this unlock.")
-            return False
-
-    def get_jenkins_job(self):
-        if self.is_locked():
-            lockfile = open(self.get_lockfile(), 'r')
-            job_name = lockfile.readline().rstrip()
-            return job_name
-        else:
-            return None
-
-    def is_locked(self):
-        return os.path.exists(self.get_lockfile())
 
=== removed file 'tests/test_MergeProposalReview.py'
--- tests/test_MergeProposalReview.py	2013-04-26 11:24:46 +0000
+++ tests/test_MergeProposalReview.py	1970-01-01 00:00:00 +0000
@@ -1,29 +0,0 @@
-import unittest
-from jlp import MergeProposalReview
-from mock import MagicMock
-
-
-class MergeProposalReviewTestCase(unittest.TestCase):
-    review = None
-    job_name = 'test-job'
-
-    def setUp(self):
-        mp = MagicMock()
-        mp.web_link = 'http://my-example-url.com'
-        self.review = MergeProposalReview(mp)
-
-    def test_is_unlocked(self):
-        self.review.unlock()
-        self.assertFalse(self.review.is_locked())
-
-    def test_is_locked(self):
-        self.review.lock(self.job_name)
-        self.assertTrue(self.review.is_locked())
-
-    def test_get_job_name(self):
-        self.review.lock(self.job_name)
-        self.assertEquals(self.review.get_jenkins_job(), self.job_name)
-
-    def test_get_job_name_with_unlocked_mp(self):
-        self.review.unlock()
-        self.assertEquals(self.review.get_jenkins_job(), None)
 
=== modified file 'tests/test_autoland.py'
--- tests/test_autoland.py	2013-05-17 15:53:01 +0000
+++ tests/test_autoland.py	2013-06-05 09:44:54 +0000
@@ -172,16 +172,10 @@
         self.branch_patch = patch('jlp.commands.autoland.Branch', new=branch)
         self.target = target
         self.Branch = self.branch_patch.start()
-        self.MergeProposalReview = MagicMock()
-        self.merge_proposal_review_patch = patch(
-            'jlp.commands.autoland.MergeProposalReview',
-            new=MagicMock(return_value=self.MergeProposalReview))
-        self.merge_proposal_review_patch.start()
 
     def tearDown(self):
         super(TestAutolandForMergeAndCommit, self).tearDown()
         self.branch_patch.stop()
-        self.merge_proposal_review_patch.stop()
 
 
 class TestAutolandMergeAndCommit(TestAutolandForMergeAndCommit):
@@ -194,7 +188,6 @@
             self.assertTrue(self.target.merge.call_count == 1)
             self.assertTrue(self.target.commit.call_count == 1)
             self.assertTrue(close_bugs_mock.call_count == 1)
-            self.MergeProposalReview.unlock.assert_called_once_with()
 
 
 class TestAutolandVoting(TestAutolandForMergeAndCommit):
@@ -244,7 +237,6 @@
             self.assertTrue(self.target.merge.call_count == 1)
             self.assertTrue(self.target.commit.call_count == 1)
             self.assertTrue(close_bugs_mock.call_count == 1)
-            self.MergeProposalReview.unlock.assert_called_once_with()
 
 
 class TestAutolandWithMultipleDputs(TestAutolandForMergeAndCommit):
@@ -326,7 +318,6 @@
             self.assertTrue(self.target.merge.call_count == 1)
             self.assertTrue(self.target.commit.call_count == 1)
             self.assertTrue(close_bugs_mock.call_count == 1)
-            self.MergeProposalReview.unlock.assert_called_once_with()
             self.assertTrue(self.dputRunner.dput.call_count == 1)
 
 
@@ -347,15 +338,11 @@
         super(TestAutolandMergeAndCommitMergeConflict, self).tearDown()
         self.branch_patch.stop()
 
-    @patch('jlp.commands.autoland.MergeProposalReview')
-    def test_merge_and_commit_merge_conflict(self, MergeProposalReview):
+    def test_merge_and_commit_merge_conflict(self):
         sys_argv = ['autoland.py', '-r', 'PASSED',
                     '-v', '123', '-m', 'url']
-        review = MagicMock()
-        MergeProposalReview.return_value = review
         with patch('sys.argv', sys_argv):
             self.assertTrue(autoland.autoland() == 1)
-            self.assertEqual(review.unlock.call_count, 1)
 
 
 class TestAutolandMergeAndCommitPushFailed(TestAutoland):
@@ -374,15 +361,11 @@
         super(TestAutolandMergeAndCommitPushFailed, self).tearDown()
         self.branch_patch.stop()
 
-    @patch('jlp.commands.autoland.MergeProposalReview')
-    def test_merge_and_commit_push_error(self, MergeProposalReview):
+    def test_merge_and_commit_push_error(self):
         sys_argv = ['autoland.py', '-r', 'PASSED',
                     '-v', '123', '-m', 'url']
-        review = MagicMock()
-        MergeProposalReview.return_value = review
         with patch('sys.argv', sys_argv):
             self.assertTrue(autoland.autoland() == 1)
-            self.assertEqual(review.unlock.call_count, 1)
 
 
 class TestAutolandMergeAndCommitLockFailed(TestAutoland):
@@ -401,15 +384,11 @@
         super(TestAutolandMergeAndCommitLockFailed, self).tearDown()
         self.branch_patch.stop()
 
-    @patch('jlp.commands.autoland.MergeProposalReview')
-    def test_merge_and_commit_lock_failed(self, MergeProposalReview):
+    def test_merge_and_commit_lock_failed(self):
         sys_argv = ['autoland.py', '-r', 'PASSED',
                     '-v', '123', '-m', 'url']
-        review = MagicMock()
-        MergeProposalReview.return_value = review
         with patch('sys.argv', sys_argv):
             self.assertTrue(autoland.autoland() == 1)
-            self.assertEqual(review.unlock.call_count, 1)
 
 
 class TestAutolandMergeAndCommitUnapprovedChange(TestAutoland):
 
=== modified file 'tests/test_jenkinsutils.py'
--- tests/test_jenkinsutils.py	2013-05-20 11:44:39 +0000
+++ tests/test_jenkinsutils.py	2013-06-05 09:44:54 +0000
@@ -70,6 +70,476 @@
                       "%s" % (err.exception))
 
 
+class TestIsJobOrDownstreamBuilding(unittest.TestCase):
+    def setUp(self):
+        self.get_config_option_patch = patch(
+            'jlp.jenkinsutils.get_config_option', new=lambda option: '')
+        self.get_config_option_patch.start()
+        self.get_json_jenkins = self.get_json_jenkins_patch = patch(
+            'jlp.jenkinsutils.get_json_jenkins')
+        self.get_json_jenkins_patch.start()
+
+    def tearDown(self):
+        self.get_config_option_patch.stop()
+        self.get_json_jenkins_patch.stop()
+
+    @patch('jlp.jenkinsutils.get_running_builds',
+           new=lambda job, job_params: [1])
+    def test_job_is_building(self):
+        self.assertTrue(
+            jenkinsutils.is_job_or_downstream_building('job', {}))
+
+    @patch('jlp.jenkinsutils.get_running_builds',
+           new=lambda job, job_params: [])
+    @patch('jlp.jenkinsutils.is_job_queued',
+           new=lambda job, queue, job_params: True)
+    def test_job_is_queued(self):
+        self.assertTrue(
+            jenkinsutils.is_job_or_downstream_building('job', {}))
+
+    @patch('jlp.jenkinsutils.get_running_builds',
+           new=lambda x, job_params: [])
+    @patch('jlp.jenkinsutils.get_downstream_projects', new=lambda x, z: [1])
+    def test_downstream_job_is_queued(self):
+        def is_job_queued(job, queue, job_params={}, upstream_job=None,
+                          upstream_params={}):
+            if upstream_job:
+                return True
+            return False
+        with patch('jlp.jenkinsutils.is_job_queued', new=is_job_queued):
+            self.assertTrue(jenkinsutils.is_job_or_downstream_building(
+                'job', {}))
+
+
+class TestIsJobOrDownstreamBuildingScenarios(TestWithScenarios):
+    scenarios = [
+        ('downstream_job_building_no_params_check',
+         {'expected': True,
+          'job_params': {},
+          'jenkins_data':  {
+              'builds': [{
+                  'building': True,
+                  'actions': [{
+                      'causes': [{
+                          'upstreamProject': 'job',
+                          'upstreamBuild': 22
+                      }]
+                  }],
+              'result': None}],
+              'upstreamProjects': [{
+                  'name': 'job',
+                  'builds': [{
+                      'number': 22,
+                      'actions': [{'parameters': [{
+                          'name': 'merge_proposal',
+                          'value': 'http://mp3'}]}]}]}]}}),
+        ('downstream_job_building_with_params_check',
+         {'expected': True,
+          'job_params': {'merge_proposal': 'http://mp'},
+          'jenkins_data':  {
+              'builds': [{
+                  'building': True,
+                  'actions': [{
+                      'causes': [{
+                          'upstreamProject': 'job',
+                          'upstreamBuild': 22
+                      }]
+                  }],
+              'result': None}],
+              'upstreamProjects': [{
+                  'name': 'job',
+                  'builds': [
+                      {
+                          'number': 23,
+                          'actions': [{'parameters': [{
+                              'name': 'merge_proposal',
+                              'value': 'http://different-mp'}]}]},
+                      {
+                          'number': 22,
+                          'actions': [{'parameters': [{
+                              'name': 'merge_proposal',
+                              'value': 'http://mp'}]}]}
+                  ]}]}}),
+        ('downstream_job_building_with_different_mp',
+         {'expected': False,
+          'job_params': {'merge_proposal': 'http://mp'},
+          'jenkins_data':  {
+              'builds': [{
+                  'building': True,
+                  'actions': [{
+                      'causes': [{
+                          'upstreamProject': 'job',
+                          'upstreamBuild': 22
+                      }]
+                  }],
+              'result': None}],
+              'upstreamProjects': [{
+                  'name': 'job',
+                  'builds': [
+                      {
+                          'number': 22,
+                          'actions': [{'parameters': [{
+                              'name': 'merge_proposal',
+                              'value': 'http://different-mp'}]}]}
+                  ]}]}}),
+
+        ('no_action',
+         {'expected': False,
+          'job_params': {},
+          'jenkins_data':  {
+              'builds': [{
+              'result': None
+              }]}}),
+        ('different_upstream',
+         {'expected': False,
+          'job_params': {},
+          'jenkins_data':  {
+              'builds': [{
+                  'building': True,
+                  'actions': [{
+                      'causes': [{
+                          'upstreamProject': 'different_job',
+                          'upstreamBuild': 23
+                      }]
+                  }],
+              'result': None
+              }]}}),
+        ('no_causes',
+         {'expected': False,
+          'job_params': {},
+          'jenkins_data':  {
+              'builds': [{
+                  'actions': [{
+                  }],
+              'result': None
+              }]}}),
+        ('no_upstream',
+         {'expected': False,
+          'job_params': {},
+          'jenkins_data':  {
+              'builds': [{
+                  'building': True,
+                  'actions': [{
+                      'causes': [{
+                      }]
+                  }],
+              'result': None
+              }]}}),
+    ]
+
+    @patch('jlp.jenkinsutils.get_config_option', new=lambda x: '')
+    @patch('jlp.jenkinsutils.get_running_builds',
+           new=lambda job, job_params={}: [])
+    @patch('jlp.jenkinsutils.get_downstream_projects',
+           new=lambda x, z: ['downstream_job'])
+    @patch('jlp.jenkinsutils.is_job_queued',
+           new=lambda x, y, upstream_job=None,
+           job_params={}, upstream_params={}: False)
+    def test_is_job_or_downstream_building(self):
+
+        jenkins = MagicMock()
+        jenkins.get_json_data = lambda x, append_api=True: self.jenkins_data
+        with patch('jlp.jenkinsutils.get_json_jenkins',
+                   new=MagicMock(return_value=jenkins)):
+            self.assertEquals(
+                jenkinsutils.is_job_or_downstream_building(
+                    'job', self.job_params),
+                self.expected)
+
+
+class TestGetRunningBuilds(TestWithScenarios):
+    scenarios = [
+        ('single_job_running',
+         {'expected': [1],
+          'job_params': {},
+          'jenkins_data':  {
+              'builds': [{
+                  'building': True,
+                  'number': 1,
+                  'actions': []
+              }]}}),
+        ('single_job_running_different_params',
+         {'expected': [],
+          'job_params': {'merge_proposal': 'http://mp'},
+          'jenkins_data':  {
+              'builds': [{
+                  'building': True,
+                  'number': 1,
+                  'actions': [{
+                      'parameters': [{
+                          'name': 'merge_proposal',
+                          'value': 'http://different-mp'
+                      }]}]
+              }]}}),
+        ('single_job_running_same_params',
+         {'expected': [1],
+          'job_params': {'merge_proposal': 'http://mp'},
+          'jenkins_data':  {
+              'builds': [{
+                  'building': True,
+                  'number': 1,
+                  'actions': [{
+                      'parameters': [{
+                          'name': 'merge_proposal',
+                          'value': 'http://mp'
+                      }]}]
+              }]}}),
+        ('two_jobs_running',
+         {'expected': [1, 2],
+          'job_params': {},
+          'jenkins_data':  {
+              'builds': [{
+                  'building': True,
+                  'actions': [],
+                  'number': 1},
+                  {'building': True,
+                   'actions': [],
+                   'number': 2}
+              ]}}),
+        ('two_jobs_running_only_one_matches',
+         {'expected': [2],
+          'job_params': {'merge_proposal': 'http://mp'},
+          'jenkins_data':  {
+              'builds': [{
+                  'building': True,
+                  'actions': [],
+                  'number': 1},
+                  {'building': True,
+                   'actions': [{
+                       'parameters': [{
+                           'name': 'merge_proposal',
+                           'value': 'http://mp'
+                       }]}],
+                   'number': 2}
+              ]}}),
+        ('one_job_running_out_of_two',
+         {'expected': [2],
+          'job_params': {},
+          'jenkins_data':  {
+              'builds': [{
+                  'building': False,
+                  'actions': [],
+                  'number': 1},
+                  {'building': True,
+                   'actions': [],
+                   'number': 2}
+              ]}}),
+        ('no_job_running',
+         {'expected': [],
+          'job_params': {},
+          'jenkins_data':  {
+              'builds': [{
+                  'building': False,
+                  'actions': [],
+                  'number': 1},
+                  {'building': False,
+                   'actions': [],
+                   'number': 2}
+              ]}}),
+        ('no_builds',
+         {'expected': [],
+          'job_params': {},
+          'jenkins_data':  {}}),
+    ]
+
+    @patch('jlp.jenkinsutils.get_config_option', new=lambda x: '')
+    def test_get_running_builds(self):
+
+        jenkins = MagicMock()
+        jenkins.get_json_data = lambda x, append_api=True: self.jenkins_data
+        with patch('jlp.jenkinsutils.get_json_jenkins',
+                   new=MagicMock(return_value=jenkins)):
+            self.assertEquals(
+                jenkinsutils.get_running_builds('job', self.job_params),
+                self.expected)
+
+
+class TestIsJobQueued(TestWithScenarios):
+    scenarios = [
+        ('no_items',
+         {'expected': False,
+          'upstream_job': None,
+          'job_params': {},
+          'queue': {'items': []}}),
+        ('different_job_in_queue',
+         {'expected': False,
+          'upstream_job': None,
+          'job_params': {},
+          'queue': {'items': [{
+              'task': {'name': 'different-job'}
+          }]}}),
+        ('no_upstream_job_and_match',
+         {'expected': True,
+          'upstream_job': None,
+          'job_params': {},
+          'queue': {'items': [{
+              'task': {'name': 'job'}
+          }]}}),
+        ('no_upstream_job_and_param_match',
+         {'expected': True,
+          'upstream_job': None,
+          'job_params': {'merge_proposal': 'http://mp'},
+          'queue': {'items': [{
+              'task': {'name': 'job'},
+              'actions': [{
+                  'parameters': [{
+                      'name': 'merge_proposal',
+                      'value': 'http://mp'}]}]
+          }]}}),
+        ('no_upstream_job_and_param_mismatch',
+         {'expected': False,
+          'upstream_job': None,
+          'job_params': {'merge_proposal': 'http://mp'},
+          'queue': {'items': [{
+              'task': {'name': 'job'},
+              'actions': [{
+                  'parameters': [{
+                      'name': 'merge_proposal',
+                      'value': 'http://different-mp'}]}]
+          }]}}),
+        ('no_actions',
+         {'expected': False,
+          'upstream_job': 'upstream-job',
+          'job_params': {},
+          'queue': {'items': [{
+              'task': {'name': 'job'}
+          }]}}),
+        ('no_causes',
+         {'expected': False,
+          'upstream_job': 'upstream-job',
+          'job_params': {},
+          'queue': {'items': [{
+              'task': {'name': 'job'},
+              'actions': [{}]
+          }]}}),
+        ('triggered_by_upstream',
+         {'expected': True,
+          'upstream_job': 'upstream-job',
+          'job_params': {},
+          'queue': {'items': [{
+              'task': {'name': 'job'},
+              'actions': [{
+                  'causes': [{
+                      'upstreamProject': 'upstream-job',
+                      'upstreamBuild': 22,
+                  }]
+              }]
+          }]}}),
+        ('triggered_by_upstream_with_the_right_mp',
+         {'expected': True,
+          'upstream_job': 'upstream-job',
+          'job_params': {},
+          'upstream_params': {'merge_proposal': 'http://mp'},
+          'get_build_actions': [{
+              'parameters': [{
+                  'name': 'merge_proposal',
+                  'value': 'http://mp'
+              }]}],
+          'queue': {'items': [{
+              'task': {'name': 'job'},
+              'actions': [{
+                  'causes': [{
+                      'upstreamProject': 'upstream-job',
+                      'upstreamBuild': 22,
+                  }]
+              }]
+          }]}}),
+        ('triggered_by_upstream_with_no_params',
+         {'expected': False,
+          'upstream_job': 'upstream-job',
+          'job_params': {},
+          'upstream_params': {'merge_proposal': 'http://mp'},
+          'get_build_actions': [{}],
+          'queue': {'items': [{
+              'task': {'name': 'job'},
+              'actions': [{
+                  'causes': [{
+                      'upstreamProject': 'upstream-job',
+                      'upstreamBuild': 22,
+                  }]
+              }]
+          }]}}),
+
+        ('triggered_by_upstream_but_different_mp',
+         {'expected': False,
+          'upstream_job': 'upstream-job',
+          'job_params': {},
+          'upstream_params': {'merge_proposal': 'http://mp'},
+          'get_build_actions': [{
+              'parameters': [{
+                  'name': 'merge_proposal',
+                  'value': 'http://different-mp'
+              }]}],
+          'queue': {'items': [{
+              'task': {'name': 'job'},
+              'actions': [{
+                  'causes': [{
+                      'upstreamProject': 'upstream-job',
+                      'upstreamBuild': 22,
+                  }]
+              }]
+          }]}}),
+        ('triggered_by_different_upstream',
+         {'expected': False,
+          'upstream_job': 'upstream-job',
+          'job_params': {},
+          'queue': {'items': [{
+              'task': {'name': 'job'},
+              'actions': [{
+                  'causes': [{
+                      'upstreamProject': 'different-upstream-job',
+                      'upstreamBuild': 23
+                  }]
+              }]
+          }]}}),
+        ('no_queue_provided',
+         {'expected': False,
+          'upstream_job': 'upstream-job',
+          'job_params': {},
+          'queue': None}),
+
+    ]
+
+    @patch('jlp.jenkinsutils._get_build_actions')
+    @patch('jlp.jenkinsutils.get_json_jenkins')
+    def test_is_job_queued(self, get_json_jenkins, get_build_actions):
+        json_jenkins = MagicMock()
+        json_jenkins.get_json_data.return_value = {'items': []}
+        get_json_jenkins.return_value = json_jenkins
+
+        if 'get_build_actions' in dir(self):
+            get_build_actions.return_value = self.get_build_actions
+        if 'upstream_params' in dir(self):
+            upstream_params = self.upstream_params
+        else:
+            upstream_params = {}
+        self.assertEquals(
+            jenkinsutils.is_job_queued(
+                'job', queue=self.queue, job_params=self.job_params,
+                upstream_job=self.upstream_job,
+                upstream_params=upstream_params),
+            self.expected)
+
+
+class TestGetBuildActions(unittest.TestCase):
+    @patch('jlp.jenkinsutils.get_json_jenkins')
+    def test_no_actions(self, get_json_jenkins):
+        json_jenkins = MagicMock()
+        json_jenkins.get_json_data.return_value = {}
+        get_json_jenkins.return_value = json_jenkins
+        self.assertEquals(jenkinsutils._get_build_actions('job', 23),
+                          [])
+
+    @patch('jlp.jenkinsutils.get_json_jenkins')
+    def test_actions_are_returned(self, get_json_jenkins):
+        actions = object()
+        json_jenkins = MagicMock()
+        json_jenkins.get_json_data.return_value = {'actions': actions}
+        get_json_jenkins.return_value = json_jenkins
+        ret = jenkinsutils._get_build_actions('job', 23)
+        self.assertTrue(actions == ret)
+
+
 class JenkinsUtilsGetDownstreamBuilds(unittest.TestCase):
 
     def setUp(self):
@@ -663,7 +1133,8 @@
         jenkinsutils.trigger_ci_build(lp_handle, [], 'job', 'url')
         self.assertEqual(start_jenkins_job.call_count, 0)
 
-    @patch('jlp.launchpadutils.testing_in_progress', new=lambda x: True)
+    @patch('jlp.launchpadutils.testing_in_progress',
+           new=lambda mp, jenkins_job: True)
     @patch('jlp.launchpadutils.user_allowed_to_trigger_jobs')
     @patch('jlp.jenkinsutils.start_jenkins_job')
     def test_trigger_ci_while_testing_in_progress(self,
@@ -676,7 +1147,8 @@
 
     @patch('jlp.launchpadutils.latest_candidate_validated',
            new=lambda x, y: True)
-    @patch('jlp.launchpadutils.testing_in_progress', new=lambda x: False)
+    @patch('jlp.launchpadutils.testing_in_progress',
+           new=lambda mp, jenkins_job: False)
     @patch('jlp.launchpadutils.user_allowed_to_trigger_jobs')
     @patch('jlp.jenkinsutils.start_jenkins_job')
     def test_trigger_ci_while_Revision_validated(self,
@@ -690,7 +1162,8 @@
 
     @patch('jlp.launchpadutils.latest_candidate_validated',
            new=lambda x, y: False)
-    @patch('jlp.launchpadutils.testing_in_progress', new=lambda x: False)
+    @patch('jlp.launchpadutils.testing_in_progress',
+           new=lambda mp, jenkins_job: False)
     @patch('jlp.launchpadutils.user_allowed_to_trigger_jobs')
     @patch('jlp.jenkinsutils.start_jenkins_job')
     def test_trigger_ci_while_Revision_not_validated(self,
@@ -743,7 +1216,7 @@
     @patch('jlp.launchpadutils.user_allowed_to_trigger_jobs',
            new=lambda x: True)
     @patch('jlp.launchpadutils.testing_in_progress',
-           new=lambda mp: True)
+           new=lambda mp, jenkins_job: True)
     @patch('jlp.jenkinsutils.start_jenkins_job')
     def test_trigger_al_while_testing_in_progress(self, start_jenkins_job):
         """When testing is in progress jenkins job must not be started"""
@@ -756,7 +1229,7 @@
     @patch('jlp.launchpadutils.unapproved_prerequisite_exists',
            new=lambda x: True)
     @patch('jlp.launchpadutils.testing_in_progress',
-           new=lambda x: False)
+           new=lambda x, y: False)
     @patch('jlp.launchpadutils.is_commit_message_set',
            new=lambda x, y, z: True)
     @patch('jlp.launchpadutils.user_allowed_to_trigger_jobs',
@@ -777,7 +1250,7 @@
     @patch('jlp.launchpadutils.unapproved_prerequisite_exists',
            new=lambda x: False)
     @patch('jlp.launchpadutils.testing_in_progress',
-           new=lambda x: False)
+           new=lambda x, y: False)
     @patch('jlp.launchpadutils.user_allowed_to_trigger_jobs',
            new=lambda x: True)
     @patch('jlp.jenkinsutils.start_jenkins_job')
@@ -804,6 +1277,10 @@
         self.start_jenkins_job_patch = patch(
             'jlp.jenkinsutils.start_jenkins_job')
         self.start_jenkins_job_patch.start()
+        self.unapproved_prerequisite_exists_patch = patch(
+            'jlp.launchpadutils.unapproved_prerequisite_exists',
+            new=lambda mp: False)
+        self.unapproved_prerequisite_exists_patch.start()
         #return True so we stop the flow; We just want to test the permissions
         #above and not the whole trigger_al_build method
         test_in_progress = MagicMock(return_value=True)
@@ -817,6 +1294,7 @@
         self.testing_in_progress_patch.stop()
         self.user_allowed_to_trigger_jobs_patch.stop()
         self.start_jenkins_job_patch.stop()
+        self.unapproved_prerequisite_exists_patch.stop()
 
     def test_trigger_al_with_trusted_reviewer(self):
         mp = MagicMock()
 
=== modified file 'tests/test_launchpadutils.py'
--- tests/test_launchpadutils.py	2013-05-10 14:10:52 +0000
+++ tests/test_launchpadutils.py	2013-06-05 09:44:54 +0000
@@ -1,5 +1,5 @@
 import unittest
-from jlp import launchpadutils, MergeProposalReview, get_config_option
+from jlp import launchpadutils, get_config_option
 from mock import MagicMock, patch
 from testscenarios import TestWithScenarios
 from textwrap import dedent
@@ -301,27 +301,26 @@
         self.assertTrue(ret)
         self._cleanup()
 
+    @patch('jlp.launchpadutils.jenkinsutils.is_job_or_downstream_building',
+           new=lambda jenkins_job, job_params: False)
     def test_testing_not_in_progress_approved(self):
-        """ If there is no MergeProposalReview lock then testing_in_progress
+        """ If there is no job running then testing_in_progress
         must return False.
         """
         mp = MagicMock()
         mp.web_link = 'http://my-example-url.com'
-        review = MergeProposalReview(mp)
-        review.unlock()
-        self.assertFalse(launchpadutils.testing_in_progress(mp))
+        self.assertFalse(launchpadutils.testing_in_progress(mp, 'job'))
 
+    @patch('jlp.launchpadutils.jenkinsutils.is_job_or_downstream_building',
+           new=lambda jenkins_job, job_params: True)
     def test_testing_not_in_progress_abstain(self):
-        """ If a MergeProposalReview lock exists then testing_in_progress
-        must return True and it must report the correct jenkins job which
-        holds the lock.
+        """ If there is job running
+        then testing_in_progress must return True and it must report the
+        correct jenkins job which holds the lock.
         """
         mp = MagicMock()
         mp.web_link = 'http://my-example-url.com'
-        review = MergeProposalReview(mp)
-        review.lock('my-job')
-        self.assertTrue(launchpadutils.testing_in_progress(mp))
-        self.assertEquals(review.get_jenkins_job(), 'my-job')
+        self.assertTrue(launchpadutils.testing_in_progress(mp, 'job'))
 
 
 class TestLatestReview(unittest.TestCase):
 
=== added file 'tests/test_socketlock.py'
--- tests/test_socketlock.py	1970-01-01 00:00:00 +0000
+++ tests/test_socketlock.py	2013-06-05 09:44:54 +0000
@@ -0,0 +1,117 @@
+from testtools import TestCase
+from testtools.matchers import Equals
+from jlp import SocketLock
+import socket
+from lockfile import LockTimeout, AlreadyLocked
+from multiprocessing import Process
+from autopilot.matchers import Eventually
+import sys
+import os
+import time
+from jlp import get_config_option
+
+LOCK_STATUS_ACQUIRED = 0
+LOCK_STATUS_TIMEOUT = 1
+LOCK_STATUS_ALREADYLOCKED = 2
+
+
+def run_lock_process(timeout=None, sleeptime=5, kill_process=False):
+    try:
+        with SocketLock(get_config_option('lock_name'), timeout=timeout):
+            time.sleep(sleeptime)
+            if kill_process:
+                os.kill(os.getpid(), 9)
+    except LockTimeout:
+        sys.exit(LOCK_STATUS_TIMEOUT)
+    except AlreadyLocked:
+        sys.exit(LOCK_STATUS_ALREADYLOCKED)
+    sys.exit(LOCK_STATUS_ACQUIRED)
+
+
+def is_locked():
+    try:
+        mysocket = socket.socket(socket.AF_UNIX, socket.SOCK_DGRAM)
+        mysocket.bind('\0' + get_config_option('lock_name'))
+        mysocket.close()
+        return False
+    except socket.error:
+        return True
+
+
+class TestLocking(TestCase):
+
+    def setUp(self):
+        super(TestLocking, self).setUp()
+        self.assertThat(is_locked(), Equals(False))
+
+    def test_lock_can_be_acquired(self):
+        """Create a process and check the lock exists """
+        lock_process = Process(target=run_lock_process, args=())
+        lock_process.start()
+        self.assertThat(is_locked, Eventually(Equals(True)))
+        lock_process.join()
+        self.assertThat(lock_process.exitcode, Equals(LOCK_STATUS_ACQUIRED))
+
+    def test_lock_timeouts(self):
+        """Create a process that holds the lock for 20s. Then create another
+           process that timeouts after trying to acquire the lock for 1s.
+           Make sure the timeout happens."""
+
+        # first create a process that holds the lock for 10 seconds
+        holder = Process(target=run_lock_process, args=(120, 20))
+        holder.start()
+        #give the holder some time to acquire the lock
+        self.assertThat(is_locked, Eventually(Equals(True)))
+        #now create a process that will try to acquire the lock with 1s
+        #timeout
+        acquirer = Process(target=run_lock_process, args=(1, 5))
+        acquirer.start()
+        acquirer.join()
+        self.assertThat(acquirer.exitcode, Equals(LOCK_STATUS_TIMEOUT))
+        holder.join()
+        self.assertThat(holder.exitcode, Equals(LOCK_STATUS_ACQUIRED))
+
+    def test_consecutive_holders(self):
+        """spawn 5 process each holding the lock for 1s and make sure
+           they all exit with LOCK_STATUS_ACQUIRED"""
+        holders = []
+        for i in range(0, 5):
+            holders.append(Process(target=run_lock_process, args=(120, 1)))
+            holders[i].start()
+        for i in range(0, 5):
+            holders[i].join()
+        for i in range(0, 5):
+            self.assertThat(holders[i].exitcode, Equals(LOCK_STATUS_ACQUIRED))
+
+    def test_double_locking(self):
+        """spawn 1 holder and then 5 process trying to lock the same lock
+           with 0 timeout. They should all exit with
+           LOCK_STATUS_ALREADYLOCKED"""
+        holder = Process(target=run_lock_process, args=(120, 20))
+        holder.start()
+        #give the holder some time to acquire the lock
+        self.assertThat(is_locked, Eventually(Equals(True)))
+        acquirers = []
+        for i in range(0, 5):
+            acquirers.append(Process(target=run_lock_process, args=(0, 1)))
+            acquirers[i].start()
+        for i in range(0, 5):
+            acquirers[i].join()
+        for i in range(0, 5):
+            self.assertThat(acquirers[i].exitcode,
+                            Equals(LOCK_STATUS_ALREADYLOCKED))
+        holder.join()
+        self.assertThat(holder.exitcode, Equals(LOCK_STATUS_ACQUIRED))
+
+    def test_nonrunning_holder(self):
+        """Create a lock which is not released properly and check if the
+           consequtive process can reclaim it."""
+        for i in range(0, 5):
+            holder = Process(target=run_lock_process, args=(120, 1, True))
+            holder.start()
+            holder.join()
+            self.assertThat(-9, Equals(holder.exitcode))
+            holder = Process(target=run_lock_process, args=(0, 1))
+            holder.start()
+            holder.join()
+            self.assertThat(holder.exitcode, Equals(LOCK_STATUS_ACQUIRED))
 
=== removed file 'tests/test_socketlock.py'
--- tests/test_socketlock.py	2013-05-10 14:10:52 +0000
+++ tests/test_socketlock.py	1970-01-01 00:00:00 +0000
@@ -1,117 +0,0 @@
-from testtools import TestCase
-from testtools.matchers import Equals
-from jlp import SocketLock
-import socket
-from lockfile import LockTimeout, AlreadyLocked
-from multiprocessing import Process
-from autopilot.matchers import Eventually
-import sys
-import os
-import time
-from jlp import get_config_option
-
-LOCK_STATUS_ACQUIRED = 0
-LOCK_STATUS_TIMEOUT = 1
-LOCK_STATUS_ALREADYLOCKED = 2
-
-
-def run_lock_process(timeout=None, sleeptime=5, kill_process=False):
-    try:
-        with SocketLock(get_config_option('lock_name'), timeout=timeout):
-            time.sleep(sleeptime)
-            if kill_process:
-                os.kill(os.getpid(), 9)
-    except LockTimeout:
-        sys.exit(LOCK_STATUS_TIMEOUT)
-    except AlreadyLocked:
-        sys.exit(LOCK_STATUS_ALREADYLOCKED)
-    sys.exit(LOCK_STATUS_ACQUIRED)
-
-
-def is_locked():
-    try:
-        mysocket = socket.socket(socket.AF_UNIX, socket.SOCK_DGRAM)
-        mysocket.bind('\0' + get_config_option('lock_name'))
-        mysocket.close()
-        return False
-    except socket.error:
-        return True
-
-
-class TestLocking(TestCase):
-
-    def setUp(self):
-        super(TestLocking, self).setUp()
-        self.assertThat(is_locked(), Equals(False))
-
-    def test_lock_can_be_acquired(self):
-        """Create a process and check the lock exists """
-        lock_process = Process(target=run_lock_process, args=())
-        lock_process.start()
-        self.assertThat(is_locked, Eventually(Equals(True)))
-        lock_process.join()
-        self.assertThat(lock_process.exitcode, Equals(LOCK_STATUS_ACQUIRED))
-
-    def test_lock_timeouts(self):
-        """Create a process that holds the lock for 20s. Then create another
-           process that timeouts after trying to acquire the lock for 1s.
-           Make sure the timeout happens."""
-
-        # first create a process that holds the lock for 10 seconds
-        holder = Process(target=run_lock_process, args=(120, 20))
-        holder.start()
-        #give the holder some time to acquire the lock
-        self.assertThat(is_locked, Eventually(Equals(True)))
-        #now create a process that will try to acquire the lock with 1s
-        #timeout
-        acquirer = Process(target=run_lock_process, args=(1, 5))
-        acquirer.start()
-        acquirer.join()
-        self.assertThat(acquirer.exitcode, Equals(LOCK_STATUS_TIMEOUT))
-        holder.join()
-        self.assertThat(holder.exitcode, Equals(LOCK_STATUS_ACQUIRED))
-
-    def test_consecutive_holders(self):
-        """spawn 5 process each holding the lock for 1s and make sure
-           they all exit with LOCK_STATUS_ACQUIRED"""
-        holders = []
-        for i in range(0, 5):
-            holders.append(Process(target=run_lock_process, args=(120, 1)))
-            holders[i].start()
-        for i in range(0, 5):
-            holders[i].join()
-        for i in range(0, 5):
-            self.assertThat(holders[i].exitcode, Equals(LOCK_STATUS_ACQUIRED))
-
-    def test_double_locking(self):
-        """spawn 1 holder and then 5 process trying to lock the same lock
-           with 0 timeout. They should all exit with
-           LOCK_STATUS_ALREADYLOCKED"""
-        holder = Process(target=run_lock_process, args=(120, 20))
-        holder.start()
-        #give the holder some time to acquire the lock
-        self.assertThat(is_locked, Eventually(Equals(True)))
-        acquirers = []
-        for i in range(0, 5):
-            acquirers.append(Process(target=run_lock_process, args=(0, 1)))
-            acquirers[i].start()
-        for i in range(0, 5):
-            acquirers[i].join()
-        for i in range(0, 5):
-            self.assertThat(acquirers[i].exitcode,
-                            Equals(LOCK_STATUS_ALREADYLOCKED))
-        holder.join()
-        self.assertThat(holder.exitcode, Equals(LOCK_STATUS_ACQUIRED))
-
-    def test_nonrunning_holder(self):
-        """Create a lock which is not released properly and check if the
-           consequtive process can reclaim it."""
-        for i in range(0, 5):
-            holder = Process(target=run_lock_process, args=(120, 1, True))
-            holder.start()
-            holder.join()
-            self.assertThat(-9, Equals(holder.exitcode))
-            holder = Process(target=run_lock_process, args=(0, 1))
-            holder.start()
-            holder.join()
-            self.assertThat(holder.exitcode, Equals(LOCK_STATUS_ACQUIRED))