address some review comments

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

master/buildbot/test/unit/test_www_config.py

@@ -20,6 +20,7 @@
 from buildbot.www import auth
 from buildbot.www import config
 from twisted.internet import defer
+from twisted.python import log
 from twisted.python import util
 from twisted.trial import unittest
 
@@ -65,12 +66,14 @@ def test_render(self):
     def test_parseCustomTemplateDir(self):
         exp = {'views/builds.html': json.dumps('<div>\n</div>')}
         try:
+            # we make the test work if pyjade is present or note
+            # It is better than just skip if pyjade is not there
             import pyjade
             [pyjade]
             exp.update({'plugin/views/plugin.html':
                         json.dumps(u'<div class="myclass"><pre>this is customized</pre></div>')})
         except ImportError:
-            pass
+            log.msg("Only testing html based template override")
         template_dir = util.sibpath(__file__, "test_templates_dir")
         master = self.make_master(url='h:/a/b/')
         rsrc = config.IndexResource(master, "foo")

master/buildbot/www/config.py

@@ -15,6 +15,7 @@
 
 import jinja2
 import os
+import posixpath
 
 from buildbot.interfaces import IConfigured
 from buildbot.util import json
@@ -37,18 +38,16 @@ def reconfigResource(self, new_config):
         self.config = new_config.www
 
         versions = self.getEnvironmentVersions()
+        vs = self.config.get('versions')
+        if isinstance(vs, list):
+            versions += vs
+        self.config['versions'] = versions
 
-        if isinstance(self.config.get('versions'), list):
-            versions += self.config['versions']
-
-        template_dir = self.config.get('custom_templates_dir')
+        self.custom_templates = {}
+        template_dir = self.config.pop('custom_templates_dir', None)
         if template_dir is not None:
-            del self.config['custom_templates_dir']
             template_dir = os.path.join(self.master.basedir, template_dir)
             self.custom_templates = self.parseCustomTemplateDir(template_dir)
-        else:
-            self.custom_templates = {}
-        self.config['versions'] = versions
 
     def render_GET(self, request):
         return self.asyncRenderHelper(request, self.renderIndex)
@@ -64,9 +63,11 @@ def parseCustomTemplateDir(self, template_dir):
             pyjade = None
         for root, dirs, files in os.walk(template_dir):
             if root == template_dir:
-                template_name = "views/%s.html"
+                template_name = posixpath.join("views", "%s.html")
             else:
-                template_name = os.path.basename(root) + "/views/%s.html"
+                # template_name is a url, so we really want '/'
+                # root is a os.path, though
+                template_name = posixpath.join(os.path.basename(root), "views", "%s.html")
             for f in files:
                 fn = os.path.join(root, f)
                 basename, ext = os.path.splitext(f)
@@ -75,7 +76,7 @@ def parseCustomTemplateDir(self, template_dir):
                 if ext == ".html":
                     with open(fn) as f:
                         html = f.read().strip()
-                if ext == ".jade":
+                elif ext == ".jade":
                     with open(fn) as f:
                         jade = f.read()
                         parser = pyjade.parser.Parser(jade)
@@ -135,6 +136,8 @@ def toJson(obj):
             return repr(obj) + " not yet IConfigured"
 
         tpl = self.jinja.get_template('index.html')
+        # we use Jinja in order to render some server side dynamic stuff
+        # For example, custom_templates javascript is generated by the layout.jade jinja template
         tpl = tpl.render(configjson=json.dumps(config, default=toJson),
                          custom_templates=self.custom_templates,
                          config=self.config)