# HG changeset patch
# User cmlenz
# Date 1187694816 0
# Node ID 20ddfbb8e879505bce1cb77f19f72be6cba07944
# Parent  99d516f751697b8dc89d00471b1ee951cab3a5f3
Fix for #154, added unit tests.

diff --git a/bitten/tests/admin.py b/bitten/tests/admin.py
--- a/bitten/tests/admin.py
+++ b/bitten/tests/admin.py
@@ -17,6 +17,7 @@
 from trac.test import EnvironmentStub, Mock
 from trac.web.href import Href
 from trac.web.main import RequestDone
+from bitten.main import BuildSystem
 from bitten.model import BuildConfig, TargetPlatform, schema
 from bitten.admin import BuildMasterAdminPageProvider, \
                          BuildConfigurationsAdminPageProvider
diff --git a/bitten/tests/web_ui.py b/bitten/tests/web_ui.py
--- a/bitten/tests/web_ui.py
+++ b/bitten/tests/web_ui.py
@@ -12,13 +12,16 @@
 import tempfile
 import unittest
 
+from trac.core import TracError
 from trac.db import DatabaseManager
 from trac.perm import PermissionCache, PermissionSystem
 from trac.test import EnvironmentStub, Mock
+from trac.util.html import Markup
 from trac.web.clearsilver import HDFWrapper
 from trac.web.href import Href
-from bitten.model import BuildConfig, TargetPlatform, schema
-from bitten.web_ui import BuildConfigController
+from bitten.main import BuildSystem
+from bitten.model import Build, BuildConfig, BuildStep, TargetPlatform, schema
+from bitten.web_ui import BuildConfigController, SourceFileLinkFormatter
 
 
 class BuildConfigControllerTestCase(unittest.TestCase):
@@ -121,8 +124,119 @@
                              req.hdf.get('chrome.links.next.0.href'))
 
 
+class SourceFileLinkFormatterTestCase(unittest.TestCase):
+
+    def setUp(self):
+        self.env = EnvironmentStub()
+
+        # Create tables
+        db = self.env.get_db_cnx()
+        cursor = db.cursor()
+        connector, _ = DatabaseManager(self.env)._get_connector()
+        for table in schema:
+            for stmt in connector.to_sql(table):
+                cursor.execute(stmt)
+
+        # Hook up a dummy repository
+        self.repos = Mock(
+            get_node=lambda path, rev=None: Mock(get_history=lambda: [],
+                                                 isdir=True),
+            normalize_path=lambda path: path,
+            sync=lambda: None
+        )
+        self.env.get_repository = lambda authname=None: self.repos
+
+    def tearDown(self):
+        pass
+
+    def test_format_simple_link_in_repos(self):
+        BuildConfig(self.env, name='test', path='trunk').insert()
+        build = Build(self.env, config='test', platform=1, rev=123, rev_time=42,
+                      status=Build.SUCCESS, slave='hal')
+        build.insert()
+        step = BuildStep(self.env, build=build.id, name='foo',
+                         status=BuildStep.SUCCESS)
+        step.insert()
+
+        self.repos.get_node = lambda path, rev: (path, rev)
+
+        req = Mock(method='GET', href=Href('/trac'), authname='hal')
+        comp = SourceFileLinkFormatter(self.env)
+        formatter = comp.get_formatter(req, build)
+
+        output = formatter(step, None, None, u'error in foo/bar.c')
+        self.assertEqual(Markup, type(output))
+        self.assertEqual('error in <a href="/trac/browser/trunk/foo/bar.c">'
+                         'foo/bar.c</a>', output)
+
+    def test_format_simple_link_not_in_repos(self):
+        BuildConfig(self.env, name='test', path='trunk').insert()
+        build = Build(self.env, config='test', platform=1, rev=123, rev_time=42,
+                      status=Build.SUCCESS, slave='hal')
+        build.insert()
+        step = BuildStep(self.env, build=build.id, name='foo',
+                         status=BuildStep.SUCCESS)
+        step.insert()
+
+        def _raise():
+            raise TracError('No such node')
+        self.repos.get_node = lambda path, rev: _raise()
+
+        req = Mock(method='GET', href=Href('/trac'), authname='hal')
+        comp = SourceFileLinkFormatter(self.env)
+        formatter = comp.get_formatter(req, build)
+
+        output = formatter(step, None, None, u'error in foo/bar.c')
+        self.assertEqual(Markup, type(output))
+        self.assertEqual('error in foo/bar.c', output)
+
+    def test_format_link_in_repos_with_line(self):
+        BuildConfig(self.env, name='test', path='trunk').insert()
+        build = Build(self.env, config='test', platform=1, rev=123, rev_time=42,
+                      status=Build.SUCCESS, slave='hal')
+        build.insert()
+        step = BuildStep(self.env, build=build.id, name='foo',
+                         status=BuildStep.SUCCESS)
+        step.insert()
+
+        self.repos.get_node = lambda path, rev: (path, rev)
+
+        req = Mock(method='GET', href=Href('/trac'), authname='hal')
+        comp = SourceFileLinkFormatter(self.env)
+        formatter = comp.get_formatter(req, build)
+
+        output = formatter(step, None, None, u'error in foo/bar.c:123')
+        self.assertEqual(Markup, type(output))
+        self.assertEqual('error in <a href="/trac/browser/trunk/foo/bar.c#L123">'
+                         'foo/bar.c:123</a>', output)
+
+    def test_format_link_not_in_repos_with_line(self):
+        BuildConfig(self.env, name='test', path='trunk').insert()
+        build = Build(self.env, config='test', platform=1, rev=123, rev_time=42,
+                      status=Build.SUCCESS, slave='hal')
+        build.insert()
+        step = BuildStep(self.env, build=build.id, name='foo',
+                         status=BuildStep.SUCCESS)
+        step.insert()
+
+        def _raise():
+            raise TracError('No such node')
+        self.repos.get_node = lambda path, rev: _raise()
+
+        req = Mock(method='GET', href=Href('/trac'), authname='hal')
+        comp = SourceFileLinkFormatter(self.env)
+        formatter = comp.get_formatter(req, build)
+
+        output = formatter(step, None, None, u'error in foo/bar.c:123')
+        self.assertEqual(Markup, type(output))
+        self.assertEqual('error in foo/bar.c:123', output)
+
+
 def suite():
-    return unittest.makeSuite(BuildConfigControllerTestCase, 'test')
+    suite = unittest.TestSuite()
+    suite.addTest(unittest.makeSuite(BuildConfigControllerTestCase, 'test'))
+    suite.addTest(unittest.makeSuite(SourceFileLinkFormatterTestCase, 'test'))
+    return suite
 
 if __name__ == '__main__':
     unittest.main(defaultTest='suite')
diff --git a/bitten/web_ui.py b/bitten/web_ui.py
--- a/bitten/web_ui.py
+++ b/bitten/web_ui.py
@@ -23,6 +23,7 @@
     from trac.Timeline import ITimelineEventProvider
 from trac.util import escape, pretty_timedelta, format_datetime, shorten_line, \
                       Markup
+from trac.util.html import html
 from trac.web import IRequestHandler
 from trac.web.chrome import INavigationContributor, ITemplateProvider, \
                             add_link, add_stylesheet
@@ -559,11 +560,12 @@
 
 class SourceFileLinkFormatter(Component):
     """Detects references to files in the build log and renders them as links
-    to the repository browser."""
+    to the repository browser.
+    """
 
     implements(ILogFormatter)
 
-    _fileref_re = re.compile('(?P<path>[\w.-]+(?:/[\w.-]+)+)(?P<line>(:\d+))')
+    _fileref_re = re.compile('(?P<path>[\w.-]+(?:/[\w.-]+)+)(?P<line>(:\d+))?')
 
     def get_formatter(self, req, build):
         """Return the log message formatter function."""
@@ -571,6 +573,7 @@
         repos = self.env.get_repository(req.authname)
         href = req.href.browser
         cache = {}
+
         def _replace(m):
             filepath = posixpath.normpath(m.group('path').replace('\\', '/'))
             if not cache.get(filepath) is True:
@@ -578,7 +581,7 @@
                 path = ''
                 for part in parts:
                     path = posixpath.join(path, part)
-                    if not path in cache:
+                    if path not in cache:
                         try:
                             repos.get_node(posixpath.join(config.path, path),
                                            build.rev)
@@ -587,9 +590,22 @@
                             cache[path] = False
                     if cache[path] is False:
                         return m.group(0)
-            return '<a href="%s">%s</a>' % (
-                   href(config.path, filepath) + '#L' + m.group('line')[1:],
-                   m.group(0))
+            link = href(config.path, filepath)
+            if m.group('line'):
+                link += '#L' + m.group('line')[1:]
+            return Markup(html.A(m.group(0), href=link))
+
         def _formatter(step, type, level, message):
-            return self._fileref_re.sub(_replace, message)
+            buf = []
+            offset = 0
+            for mo in self._fileref_re.finditer(message):
+                start, end = mo.span()
+                if start > offset:
+                    buf.append(message[offset:start])
+                buf.append(_replace(mo))
+                offset = end
+            if offset < len(buf):
+                buf.append(message[offset:])
+            return Markup("").join(buf)
+
         return _formatter