[changes] Skip showing empty table

Summary:
I was looking at the changesbot comments and noticed this: https://tails.corp.dropbox.com/D98392#1516833

Skipping empty results table

Test Plan: unit tests

Reviewers: vishal

Reviewed By: vishal

Subscribers: wwu, alexallain

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

changes/listeners/phabricator_listener.py

@@ -185,11 +185,11 @@ def get_test_failure_remarkup(build, tests):
         new_failures=len(new_failures),
         link=build_uri('/projects/{0}/builds/{1}/tests/?result=failed'.format(build.project.slug, build.id.hex))
     )
-    message += '\n\n'
-    message += '**New failures ({new_failure_count}):**\n'.format(
-        new_failure_count=len(new_failures)
-    )
-    message += _generate_remarkup_table_for_tests(build, new_failures)
+    if new_failures:
+        message += '\n\n**New failures ({new_failure_count}):**\n'.format(
+            new_failure_count=len(new_failures)
+        )
+        message += _generate_remarkup_table_for_tests(build, new_failures)
 
     if failures_in_parent:
         message += '\n\n**Failures in parent revision ({parent_failure_count}):**\n'.format(

tests/changes/listeners/test_phabricator.py

@@ -93,6 +93,52 @@ def test_build_failure_with_tests(self, get_options, phab):
 **New failures (1):**
 |Test Name | Package|
 |--|--|
+|{2}|test.group.ClassName|"""
+
+        phab.assert_called_once_with('1', expected_msg.format(build_link, failure_link, test_desc))
+
+    @mock.patch('changes.listeners.phabricator_listener.get_test_failures_in_base_commit')
+    @mock.patch('changes.listeners.phabricator_listener.post_diff_comment')
+    @mock.patch('changes.listeners.phabricator_listener.get_options')
+    def test_no_new_failures(self, get_options, phab, get_base_failures):
+        get_options.return_value = {
+            'phabricator.notify': '1'
+        }
+        project = self.create_project(name='Server', slug='project-slug')
+        self.assertEquals(phab.call_count, 0)
+        patch = self.create_patch()
+        source = self.create_source(project, revision_sha='1235', patch=patch)
+        build = self.create_build(project, result=Result.failed, target='D1', source=source)
+        job = self.create_job(build=build)
+        testcase = self.create_test(
+            package='test.group.ClassName',
+            name='test.group.ClassName.test_foo',
+            job=job,
+            duration=134,
+            result=Result.failed,
+            )
+        get_base_failures.return_value = {testcase.name}
+
+        build_finished_handler(build_id=build.id.hex)
+
+        get_options.assert_called_once_with(project.id)
+        build_link = build_uri('/projects/{0}/builds/{1}/'.format(
+            build.project.slug, build.id.hex))
+        failure_link = build_uri('/projects/{0}/builds/{1}/tests/?result=failed'.format(
+            build.project.slug, build.id.hex))
+
+        test_link = build_uri('/projects/{0}/builds/{1}/jobs/{2}/tests/{3}/'.format(
+            build.project.slug,
+            build.id.hex,
+            testcase.job_id.hex,
+            testcase.id.hex
+        ))
+        test_desc = "[test_foo](%s)" % test_link
+        expected_msg = """Server build Failed {{icon times, color=red}} ([results]({0})). There were 0 new [test failures]({1})
+
+**Failures in parent revision (1):**
+|Test Name | Package|
+|--|--|
 |{2}|test.group.ClassName|"""
 
         phab.assert_called_once_with('1', expected_msg.format(build_link, failure_link, test_desc))