phabricator comment formatting: leave blank line after the colored he…

…ader

Summary: Otherwise the list of builds is not interpreted as a list.

Reviewers: alexmv

Reviewed By: alexmv

Subscribers: changesbot, anupc, kylec

Differential Revision: https://tails.corp.dropbox.com/D230993

changes/listeners/phabricator_listener.py

@@ -198,12 +198,12 @@ def build_finished_handler(build_id, **kwargs):
 
     message = ""
     if bad_builds:
-        message += '(IMPORTANT) Failing builds!\n'
+        message += '(IMPORTANT) Failing builds!\n\n'
         message += '\n'.join([_get_message_for_build_context(x) for x in bad_builds])
     if good_builds:
         if bad_builds:
             message += '\n\n'
-        message += '(NOTE) Passing builds:\n'
+        message += '(NOTE) Passing builds:\n\n'
         message += '\n'.join([_get_message_for_build_context(x) for x in good_builds])
 
     post_comment(target, message, phab)

tests/changes/listeners/test_phabricator.py

@@ -74,7 +74,7 @@ def test_whitelisted_project(self, get_options, post):
 
         get_options.assert_called_once_with(project.id)
         build_link = build_web_uri('/find_build/{0}/'.format(build.id.hex))
-        expected_msg = "(IMPORTANT) Failing builds!\n - [test]({0}).".format(
+        expected_msg = "(IMPORTANT) Failing builds!\n\n - [test]({0}).".format(
             build_link
         )
 
@@ -113,7 +113,7 @@ def test_build_failure_with_tests_and_no_base_build(self, get_options, post):
             testcase.id.hex
         ))
         test_desc = "[test_foo](%s)" % test_link
-        expected_msg = """(IMPORTANT) Failing builds!\n - [Server]({0}). There were a total of 1 [test failures]({1}), but we could not determine if any of these tests were previously failing.
+        expected_msg = """(IMPORTANT) Failing builds!\n\n - [Server]({0}). There were a total of 1 [test failures]({1}), but we could not determine if any of these tests were previously failing.
 
 **All failures (1):**
 |Test Name | Package|
@@ -159,7 +159,7 @@ def test_build_failure_with_tests_and_no_base_job(self, get_options, post):
             testcase.id.hex,
         ))
         test_desc = "[test_foo](%s)" % test_link
-        expected_msg = """(IMPORTANT) Failing builds!\n - [Server]({0}). There were a total of 1 [test failures]({1}), but we could not determine if any of these tests were previously failing.
+        expected_msg = """(IMPORTANT) Failing builds!\n\n - [Server]({0}). There were a total of 1 [test failures]({1}), but we could not determine if any of these tests were previously failing.
 
 **All failures (1):**
 |Test Name | Package|
@@ -204,7 +204,7 @@ def test_build_failure_with_tests(self, get_options, post):
             testcase.id.hex,
         ))
         test_desc = "[test_foo](%s)" % test_link
-        expected_msg = """(IMPORTANT) Failing builds!\n - [Server]({0}). There were 1 new [test failures]({1})
+        expected_msg = """(IMPORTANT) Failing builds!\n\n - [Server]({0}). There were 1 new [test failures]({1})
 
 **New failures (1):**
 |Test Name | Package|
@@ -246,7 +246,7 @@ def test_no_new_failures(self, get_options, post, get_base_failures):
             testcase.id.hex,
         ))
         test_desc = "[test_foo](%s)" % test_link
-        expected_msg = """(IMPORTANT) Failing builds!\n - [Server]({0}). There were 0 new [test failures]({1})
+        expected_msg = """(IMPORTANT) Failing builds!\n\n - [Server]({0}). There were 0 new [test failures]({1})
 
 **Failures in parent revision (1):**
 |Test Name | Package|
@@ -298,7 +298,7 @@ def get_test_desc(build, testcase, test_name):
 
         test_desc = get_test_desc(build, testcase, 'test_foo')
         test_desc2 = get_test_desc(build, testcase2, 'test_foo2')
-        expected_msg = """(IMPORTANT) Failing builds!\n - [Server]({0}). There were 1 new [test failures]({1})
+        expected_msg = """(IMPORTANT) Failing builds!\n\n - [Server]({0}). There were 1 new [test failures]({1})
 
 **New failures (1):**
 |Test Name | Package|
@@ -402,14 +402,15 @@ def create_build(result, project):
             testcase1.id.hex,
         ))
         test_desc = "[test_foo](%s)" % test_link
-        expected_msg = """(IMPORTANT) Failing builds!\n - [Server]({0}). There were 1 new [test failures]({1})
+        expected_msg = """(IMPORTANT) Failing builds!\n\n - [Server]({0}). There were 1 new [test failures]({1})
 
 **New failures (1):**
 |Test Name | Package|
 |--|--|
 |{2}|test.group.ClassName|
 
 (NOTE) Passing builds:
+
  - [Server2]({3})."""
 
         post.assert_called_once_with('1', expected_msg.format(build_link, failure_link, test_desc, build2_link), mock.ANY)
@@ -512,7 +513,7 @@ def test_slug_escape(self, get_options, post):
         get_options.assert_called_once_with(project.id)
         build_link = build_web_uri('/find_build/{0}/'.format(build.id.hex))
 
-        expected_msg = '(NOTE) Passing builds:\n - [Server]({0}).'
+        expected_msg = '(NOTE) Passing builds:\n\n - [Server]({0}).'
         post.assert_called_once_with('1', expected_msg.format(build_link), mock.ANY)
 
     @mock.patch('changes.utils.phabricator_utils.post_diff_comment')