diff --git a/contrib/buildbot/server.py b/contrib/buildbot/server.py --- a/contrib/buildbot/server.py +++ b/contrib/buildbot/server.py @@ -840,7 +840,7 @@ buildLog = tc.getBuildLog(buildId) for line in tc.getIgnoreList(): # Skip empty lines and comments in the ignore file - if not line or line[0] == '#': + if not line or line.decode().strip()[0] == '#': continue # If any of the ignore patterns match any line in the diff --git a/contrib/buildbot/test/test_endpoint_status.py b/contrib/buildbot/test/test_endpoint_status.py --- a/contrib/buildbot/test/test_endpoint_status.py +++ b/contrib/buildbot/test/test_endpoint_status.py @@ -407,6 +407,29 @@ self.phab.maniphest.edit.assert_not_called() self.slackbot.client.chat_postMessage.assert_not_called() + def test_status_master_failureAndTaskDoesNotExist_doNotIgnoreComments( + self): + data = statusRequestData() + data.buildResult = 'failure' + + self.setup_master_failureAndTaskDoesNotExist(userSearchFields={ + 'username': 'author-phab-username', + 'custom.abc:slack-username': '', + }) + self.slackbot.client.users_list.return_value = test.mocks.slackbot.users_list( + total=2) + # Make sure comment patterns do not give false positives + self.teamcity.getIgnoreList.return_value = [b'# TOTAL', b' # TOTAL'] + + response = self.app.post('/status', headers=self.headers, json=data) + assert response.status_code == 200 + self.phab.differential.revision.edit.assert_not_called() + + # Despite '# TOTAL' being in the build log, the failure was NOT ignored + # since the ignore pattern is a comment. + self.phab.maniphest.edit.assert_called() + self.slackbot.client.chat_postMessage.assert_called() + def test_status_master_failureAndTaskDoesNotExist_authorDefaultName(self): data = statusRequestData() data.buildResult = 'failure' @@ -715,7 +738,8 @@ ] for pattern in testPatterns: self.teamcity.getIgnoreList.return_value = [ - b'# Some comment followed by an empty line', + b'# Some comment', + b' # Another comment followed by an empty line', b'', pattern, ]