From a176ec483bcf1ce4894f1e90d4b9850774070551 Mon Sep 17 00:00:00 2001 From: Sean Dague Date: Sat, 30 Nov 2013 07:41:05 -0500 Subject: [PATCH] time to grow up and fit in 80 columns elastic_recheck started off life ignoring the 80 column boundary. We should stop that, as it's bad form. Also, I do multi column emacs and it blows my column widths. So fix all the E501 issues and start enforcing the rules in tox Change-Id: Ib0a1d48d085d9b21fbc1bab75e93e9cc40d36988 --- elastic_recheck/bot.py | 26 ++++++++++------- elastic_recheck/cmd/check_success.py | 3 +- elastic_recheck/elasticRecheck.py | 29 ++++++++++++++----- .../tests/functional/test_gerrit_comment.py | 16 ++++++++-- .../tests/functional/test_queries.py | 10 +++++-- .../tests/unit/test_required_files.py | 22 ++++++++++---- tox.ini | 3 +- 7 files changed, 75 insertions(+), 34 deletions(-) diff --git a/elastic_recheck/bot.py b/elastic_recheck/bot.py index 520eb26f..ebc4cde2 100755 --- a/elastic_recheck/bot.py +++ b/elastic_recheck/bot.py @@ -185,17 +185,21 @@ def _main(config): channel_config = ChannelConfig(yaml.load(open(fp))) - bot = RecheckWatchBot(channel_config.channels, - config.get('ircbot', 'nick'), - config.get('ircbot', 'pass'), - config.get('ircbot', 'server'), - config.getint('ircbot', 'port'), - config.get('ircbot', 'server_password')) - recheck = RecheckWatch(bot, channel_config, - config.get('gerrit', 'user'), - config.get('gerrit', 'query_file'), - config.get('gerrit', 'host', 'review.openstack.org'), - config.get('gerrit', 'key')) + bot = RecheckWatchBot( + channel_config.channels, + config.get('ircbot', 'nick'), + config.get('ircbot', 'pass'), + config.get('ircbot', 'server'), + config.getint('ircbot', 'port'), + config.get('ircbot', 'server_password')) + + recheck = RecheckWatch( + bot, + channel_config, + config.get('gerrit', 'user'), + config.get('gerrit', 'query_file'), + config.get('gerrit', 'host', 'review.openstack.org'), + config.get('gerrit', 'key')) recheck.start() bot.start() diff --git a/elastic_recheck/cmd/check_success.py b/elastic_recheck/cmd/check_success.py index 66395f20..df044718 100755 --- a/elastic_recheck/cmd/check_success.py +++ b/elastic_recheck/cmd/check_success.py @@ -66,7 +66,8 @@ def print_metrics(data): sorted_data = sorted(data.iteritems(), key=lambda x: -x[1]['fails']) for d in sorted_data: - print "Bug: https://bugs.launchpad.net/bugs/%s => %s" % (d[0], d[1]['query'].rstrip()) + print("Bug: https://bugs.launchpad.net/bugs/%s => %s" + % (d[0], d[1]['query'].rstrip())) get_launchpad_bug(d[0]) print "Hits" for s in d[1]['hits'].keys(): diff --git a/elastic_recheck/elasticRecheck.py b/elastic_recheck/elasticRecheck.py index 85f103ce..147580d3 100755 --- a/elastic_recheck/elasticRecheck.py +++ b/elastic_recheck/elasticRecheck.py @@ -57,6 +57,9 @@ class Stream(object): if thread: self.gerrit.startWatching() + def _is_unit_test(self, line): + return "FAILURE" in line and ("python2" in line or "pep8" in line) + def get_failed_tempest(self): self.log.debug("entering get_failed_tempest") while True: @@ -70,7 +73,7 @@ class Stream(object): self.log.debug("potential failed_tempest") found = False for line in event['comment'].split('\n'): - if "FAILURE" in line and ("python2" in line or "pep8" in line): + if self._is_unit_test(line): # Unit Test Failure found = False break @@ -137,13 +140,16 @@ class Classifier(): #Wait till Elastic search is ready self.log.debug("checking if ElasticSearch is ready") if not self._is_ready(change_number, patch_number, comment): - self.log.error("something went wrong, ElasticSearch is still not ready, " - "giving up and trying next failure") + self.log.error( + "something went wrong, ElasticSearch is still not ready, " + "giving up and trying next failure") return None self.log.debug("ElasticSearch is ready, starting to classify") bug_matches = [] for x in self.queries: - self.log.debug("Looking for bug: https://bugs.launchpad.net/bugs/%s" % x['bug']) + self.log.debug( + "Looking for bug: https://bugs.launchpad.net/bugs/%s" + % x['bug']) query = qb.single_patch(x['query'], change_number, patch_number) results = self.es.search(query, size='10') if self._urls_match(comment, results): @@ -160,7 +166,8 @@ class Classifier(): results = self.es.search(query, size='10') except pyelasticsearch.exceptions.InvalidJsonResponseError: # If ElasticSearch returns an error code, sleep and retry - #TODO(jogo): if this works pull out search into a helper function that does this. + # TODO(jogo): if this works pull out search into a helper + # function that does this. print "UHUH hit InvalidJsonResponseError" time.sleep(NUMBER_OF_RETRIES) continue @@ -170,7 +177,10 @@ class Classifier(): time.sleep(SLEEP_TIME) if i == NUMBER_OF_RETRIES - 1: return False - self.log.debug("Found hits for change_number: %s, patch_number: %s" % (change_number, patch_number)) + self.log.debug( + "Found hits for change_number: %s, patch_number: %s" + % (change_number, patch_number)) + query = qb.files_ready(change_number, patch_number) for i in range(NUMBER_OF_RETRIES): results = self.es.search(query, size='80') @@ -182,7 +192,9 @@ class Classifier(): time.sleep(SLEEP_TIME) if i == NUMBER_OF_RETRIES - 1: return False - self.log.debug("All files present for change_number: %s, patch_number: %s" % (change_number, patch_number)) + self.log.debug( + "All files present for change_number: %s, patch_number: %s" + % (change_number, patch_number)) # Just because one file is parsed doesn't mean all are, so wait a # bit time.sleep(10) @@ -249,7 +261,8 @@ def main(): print "unable to classify failure" else: for bug_number in bug_numbers: - print "Found bug: https://bugs.launchpad.net/bugs/%s" % bug_number + print("Found bug: https://bugs.launchpad.net/bugs/%s" + % bug_number) if __name__ == "__main__": main() diff --git a/elastic_recheck/tests/functional/test_gerrit_comment.py b/elastic_recheck/tests/functional/test_gerrit_comment.py index 67bfb90d..ec036c0d 100644 --- a/elastic_recheck/tests/functional/test_gerrit_comment.py +++ b/elastic_recheck/tests/functional/test_gerrit_comment.py @@ -41,7 +41,10 @@ class TestGerritComment(testtools.TestCase): result = self.gerrit.query(commit, comments=True) comments = result.get('comments') comment = comments[-1] - self.assertIn("I noticed tempest failed, I think you hit bug https://bugs.launchpad.net/bugs/1223158", comment.get('message')) + self.assertIn( + "I noticed tempest failed, I think you hit bug " + "https://bugs.launchpad.net/bugs/1223158", + comment.get('message')) def test_bugs_found(self): bug_numbers = ['1223158', '424242'] @@ -52,7 +55,11 @@ class TestGerritComment(testtools.TestCase): result = self.gerrit.query(commit, comments=True) comments = result.get('comments') comment = comments[-1] - self.assertIn("I noticed tempest failed, I think you hit bug https://bugs.launchpad.net/bugs/1223158 and https://bugs.launchpad.net/bugs/424242 and", comment.get('message')) + self.assertIn( + "I noticed tempest failed, I think you hit bug " + "https://bugs.launchpad.net/bugs/1223158 and " + "https://bugs.launchpad.net/bugs/424242 and", + comment.get('message')) def test_bug_not_found(self): project = 'gtest-org/test' @@ -62,4 +69,7 @@ class TestGerritComment(testtools.TestCase): result = self.gerrit.query(commit, comments=True) comments = result.get('comments') comment = comments[-1] - self.assertIn("https://wiki.openstack.org/wiki/GerritJenkinsGithub#Test_Failures", comment.get('message')) + self.assertIn( + "https://wiki.openstack.org/wiki/" + "GerritJenkinsGithub#Test_Failures", + comment.get('message')) diff --git a/elastic_recheck/tests/functional/test_queries.py b/elastic_recheck/tests/functional/test_queries.py index 9cbdfa55..e7d58f01 100644 --- a/elastic_recheck/tests/functional/test_queries.py +++ b/elastic_recheck/tests/functional/test_queries.py @@ -40,13 +40,16 @@ class TestQueries(tests.TestCase): def test_queries(self): for x in self.classifier.queries: - print "Looking for bug: https://bugs.launchpad.net/bugs/%s" % x['bug'] + print("Looking for bug: https://bugs.launchpad.net/bugs/%s" + % x['bug']) self.assertTrue((self._is_valid_ElasticSearch_query(x) or self._is_valid_launchpad_bug(x['bug'])), ("Something is wrong with bug %s" % x['bug'])) def _is_valid_ElasticSearch_query(self, x): - query = self.classifier._apply_template(self.classifier.general_template, x['query']) + query = self.classifier._apply_template( + self.classifier.general_template, + x['query']) results = self.classifier.es.search(query, size='10') valid_query = int(results['hits']['total']) > 0 if not valid_query: @@ -64,7 +67,8 @@ class TestQueries(tests.TestCase): bug_tasks = lp_bug.bug_tasks bug_complete = map(lambda bug_task: bug_task.is_complete, bug_tasks) projects = map(lambda bug_task: bug_task.bug_target_name, bug_tasks) - # Check if all open bug tasks are closed if is_complete is true for all tasks. + # Check if all open bug tasks are closed if is_complete is true + # for all tasks. if len(bug_complete) != bug_complete.count(True): print "bug %s is closed in launchpad" % bug return False diff --git a/elastic_recheck/tests/unit/test_required_files.py b/elastic_recheck/tests/unit/test_required_files.py index 62786c8c..d8bd4b86 100644 --- a/elastic_recheck/tests/unit/test_required_files.py +++ b/elastic_recheck/tests/unit/test_required_files.py @@ -20,16 +20,22 @@ from elastic_recheck import tests class TestRequiredFiles(tests.TestCase): def test_url(self): - url = elasticRecheck.RequiredFiles.prep_url('http://logs.openstack.org/13/46613/2/check/gate-tempest-devstack-vm-full/864bf44/console.html') - self.assertEqual(url, - 'http://logs.openstack.org/13/46613/2/check/gate-tempest-devstack-vm-full/864bf44') + url = elasticRecheck.RequiredFiles.prep_url( + 'http://logs.openstack.org/13/46613/2/check/' + 'gate-tempest-devstack-vm-full/864bf44/console.html') + self.assertEqual( + url, + 'http://logs.openstack.org/13/46613/2/check/' + 'gate-tempest-devstack-vm-full/864bf44') def _fake_urlopen(self, url): pass def test_files_at_url_pass(self): self.stubs.Set(urllib2, 'urlopen', self._fake_urlopen) - result = elasticRecheck.RequiredFiles.files_at_url('http://logs.openstack.org/13/46613/2/check/gate-tempest-devstack-vm-full/864bf44') + result = elasticRecheck.RequiredFiles.files_at_url( + 'http://logs.openstack.org/13/46613/2/check/' + 'gate-tempest-devstack-vm-full/864bf44') self.assertTrue(result) def _invalid_url_open(self, url): @@ -37,5 +43,9 @@ class TestRequiredFiles(tests.TestCase): def test_files_at_url_fail(self): self.stubs.Set(urllib2, 'urlopen', self._invalid_url_open) - self.assertFalse(elasticRecheck.RequiredFiles.files_at_url('http://logs.openstack.org/02/44502/7/check/gate-tempest-devstack-vm-neutron/4f386e5')) - self.assertFalse(elasticRecheck.RequiredFiles.files_at_url('http://logs.openstack.org/45/47445/3/check/gate-tempest-devstack-vm-full/0e43e09/')) + self.assertFalse(elasticRecheck.RequiredFiles.files_at_url( + 'http://logs.openstack.org/02/44502/7/check/' + 'gate-tempest-devstack-vm-neutron/4f386e5')) + self.assertFalse(elasticRecheck.RequiredFiles.files_at_url( + 'http://logs.openstack.org/45/47445/3/check/' + 'gate-tempest-devstack-vm-full/0e43e09/')) diff --git a/tox.ini b/tox.ini index e73f083d..dddff947 100644 --- a/tox.ini +++ b/tox.ini @@ -29,7 +29,6 @@ commands = python setup.py testr --coverage --testr-args='{posargs}' [flake8] # H803 Skipped on purpose -# E501 skipped temporarily -ignore = E123,E122,E126,E128,E501,H803 +ignore = E123,E122,E126,E128,H803 exclude=.venv,.git,.tox,dist,doc,*openstack/common*,*lib/python*,*egg,build