commentapi: provide truncate() and apply during add_comment() (and use in repo_checker).

osclib/comments.py

@@ -136,12 +136,46 @@ def add_comment(self, request_id=None, project_name=None,
         if not comment:
             raise ValueError('Empty comment.')
 
+        # Always encode as utf-8 to ensure truncate handles length properly.
+        comment = self.truncate(comment.strip().encode('utf-8'))
+
         query = {}
         if parent_id:
             query['parent_id'] = parent_id
         url = self._prepare_url(request_id, project_name, package_name, query)
         return http_POST(url, data=comment)
 
+    @staticmethod
+    def truncate(comment, suffix='...', length=65535):
+        # Handle very short length by dropping suffix and just chopping comment.
+        if length <= len(suffix) + len('\n</pre>'):
+            return comment[:length]
+        if len(comment) <= length:
+            return comment
+
+        # Determine the point at which to end by leaving room for suffix.
+        end = length - len(suffix)
+        if comment.find('<pre>', 0, end) != -1:
+            # For the sake of simplicity leave space for closing pre tag even if
+            # after truncation it may no longer be necessary. Otherwise, it
+            # requires recursion with some fun edge cases.
+            end -= len('\n</pre>')
+
+        # Check for the end location landing inside a pre tag and correct by
+        # moving in front of the tag. Landing on the ends is a noop.
+        pre_index = max(comment.rfind('<pre>', end - 4, end + 4),
+                        comment.rfind('</pre>', end - 5, end + 5))
+        if pre_index != -1:
+            end = pre_index
+
+        comment = comment[:end]
+
+        # Check for unbalanced pre tag and add a closing tag.
+        if comment.count('<pre>') > comment.count('</pre>'):
+            suffix += '\n</pre>'
+
+        return comment + suffix
+
     def delete(self, comment_id):
         """Remove a comment object.
         :param comment_id: Id of the comment object.

repo_checker.py

@@ -63,14 +63,12 @@ def package_comments(self, project):
         self.logger.info('{} package comments'.format(len(self.package_results)))
 
         for package, sections in self.package_results.items():
+            message = 'The version of this package in `{}` has installation issues and may not be installable:'.format(project)
+
             # Sort sections by text to group binaries together.
             sections = sorted(sections, key=lambda s: s.text)
-            message = '\n'.join([section.text for section in sections])
-            if len(message) > 16384:
-                # Truncate messages to avoid crashing OBS.
-                message = message[:16384 - 3] + '...'
-            message = '```\n' + message.strip() + '\n```'
-            message = 'The version of this package in `{}` has installation issues and may not be installable:\n\n'.format(project) + message
+            message += '\n\n<pre>\n{}\n</pre>'.format(
+                '\n'.join([section.text for section in sections]).strip())
 
             # Generate a hash based on the binaries involved and the number of
             # sections. This eliminates version or release changes from causing

tests/comment_tests.py

@@ -0,0 +1,48 @@
+from osclib.comments import CommentAPI
+import re
+import unittest
+
+
+class TestComment(unittest.TestCase):
+    def setUp(self):
+        self.api = CommentAPI('bogus')
+
+    def test_truncate(self):
+        comment = "string of text"
+        for i in xrange(len(comment) + 1):
+            truncated = self.api.truncate(comment, length=i)
+            print(truncated)
+            self.assertEqual(len(truncated), i)
+
+    def test_truncate_pre(self):
+        comment = """
+Some text.
+
+<pre>
+bar
+mar
+car
+</pre>
+
+## section 2
+
+<pre>
+more
+lines
+than
+you
+can
+handle
+</pre>
+""".strip()
+
+        for i in xrange(len(comment) + len('...\n</pre>')):
+            truncated = self.api.truncate(comment, length=i)
+            print('=' * 80)
+            print(truncated)
+            self.assertTrue(len(truncated) <= i, '{} <= {}'.format(len(truncated), i))
+            self.assertEqual(truncated.count('<pre>'), truncated.count('</pre>'))
+            self.assertFalse(len(re.findall(r'</?\w+[^\w>]', truncated)))
+            tag_count = truncated.count('<pre>') + truncated.count('</pre>')
+            self.assertEqual(tag_count, truncated.count('<'))
+            self.assertEqual(tag_count, truncated.count('>'))