fix review comments, and last unit test issues

Signed-off-by: Pierre Tardy <pierre.tardy@intel.com>

master/buildbot/data/properties.py

@@ -36,6 +36,8 @@ class BuildPropertiesEndpoint(base.Endpoint):
     """
 
     def get(self, resultSpec, kwargs):
+        # its not really implemented db.getBuildProperties is TBD
+        # this code is kept here as a placeholder (so its not yet documented)
         return self.master.db.builds.getBuildProperties(kwargs['buildid'])
 
 

master/buildbot/data/steps.py

@@ -99,6 +99,11 @@ def startConsuming(self, callback, options, kwargs):
             raise NotImplementedError("cannot consume from this path")
 
 
+class UrlType(types.Entity):
+    name = types.String()
+    url = types.String()
+
+
 class Step(base.ResourceType):
 
     name = "step"
@@ -121,7 +126,11 @@ class EntityType(types.Entity):
         complete_at = types.NoneOk(types.DateTime())
         results = types.NoneOk(types.Integer())
         state_strings = types.List(of=types.String())
-        urls = types.List(of=types.String())
+        urls = types.List(
+            of=types.Dict(
+                name=types.String(),
+                url=types.String()
+            ))
         link = types.Link()
     entityType = EntityType(name)
 

master/buildbot/db/steps.py

@@ -131,8 +131,9 @@ def addURL(self, stepid, name, url, _racehook=None):
         # at the end of a step)
         # this race condition is only inside the same master, as only one master
         # is supposed to add urls to a buildstep.
-        # so threading.lock is used, as we are in the thread poll
+        # so threading.lock is used, as we are in the thread pool
         if self.url_lock is None:
+            # this runs in reactor thread, so no race here..
             self.url_lock = defer.DeferredLock()
 
         def thd(conn):

master/buildbot/process/buildstep.py

@@ -351,7 +351,9 @@ def setProgress(self, metric, value):
 
     @defer.inlineCallbacks
     def setStateStrings(self, strings):
-        yield self.master.data.updates.setStepStateStrings(self.stepid, map(unicode,strings))
+        # we render the strings with properties first
+        strings = yield self.build.render(strings)
+        yield self.master.data.updates.setStepStateStrings(self.stepid, map(unicode, strings))
         # call to the status API for now
         self.step_status.old_setText(strings)
         self.step_status.old_setText2(strings)

master/docs/developer/database.rst

@@ -335,7 +335,7 @@ steps
     * ``complete_at`` (datetime at which this step finished, or None if it is ongoing)
     * ``state_strings`` (list of short strings describing the step's state)
     * ``results`` (results of this step; see :ref:`Build-Result-Codes`)
-    * ``urls`` (list of URLs produced by this step)
+    * ``urls`` (list of URLs produced by this step. Each urls is stored as a dictionary with keys `name` and `url`)
 
     .. py:method:: getStep(stepid=None, buildid=None, number=None, name=None)
 
@@ -391,6 +391,16 @@ steps
 
             This update is done unconditionally, even if the steps are already finished.
 
+    .. py:method:: addURL(self, stepid, name, url)
+
+        :param integer stepid: the stepid to add the url.
+        :param string name: the url name
+        :param string url: the actual url
+        :returns: None via deferred
+
+        Add a new url to a step.
+        The new url is added to the list of urls.
+
 logs
 ~~~~
 

master/docs/developer/rtype-step.rst

@@ -16,7 +16,7 @@ Steps
     :attr list state_strings: a list of strings giving progressively more detail on the state of the build.
         The first is usually one word or phrase; the remainder are sized for one-line display.
     :attr urls: a list of URLs associated with this step.
-    :type urls: list of strings
+    :type urls: list of dictionaries with keys `name` and `url`
     :attr Link link: link for this step
 
     This resource type describes a step in a build.
@@ -102,6 +102,16 @@ All update methods are available as attributes of ``master.data.updates``.
 
         Replace the existing state strings for a step with a new list.
 
+    .. py:method:: addStepURL(stepid, name, url):
+
+        :param integer stepid: the step to modify
+        :param string name: the url name
+        :param string url: the actual url
+        :returns: None via deferred
+
+        Add a new url to a step.
+        The new url is added to the list of urls.
+
     .. py:method:: finishStep(stepid, results)
 
         :param integer stepid: the step to modify