jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/428127 )

Change subject: [IMPR] reduce code complexity of add_text ......................................................................

[IMPR] reduce code complexity of add_text

- Reduce cyclomatic complexity of update_page from E33 to D22 and improve readability by splitting code in parts.

Change-Id: I42e61ea9a0391aafe714b52e3f3418fc93c395d7 --- M scripts/add_text.py M tests/add_text_tests.py 2 files changed, 96 insertions(+), 76 deletions(-)

Approvals: Framawiki: Looks good to me, approved jenkins-bot: Verified

diff --git a/scripts/add_text.py b/scripts/add_text.py index c3e1d74..22c8069 100755 --- a/scripts/add_text.py +++ b/scripts/add_text.py @@ -57,7 +57,7 @@

# # (C) Filnik, 2007-2010 -# (C) Pywikibot team, 2007-2017 +# (C) Pywikibot team, 2007-2018 # # Distributed under the terms of the MIT license. # @@ -79,6 +79,57 @@ }

+def get_text(page, old, create): + """Get the old text.""" + if old is None: + try: + text = page.get() + except pywikibot.NoPage: + if create: + pywikibot.output( + "{} doesn't exist, creating it!".format(page.title())) + text = '' + else: + pywikibot.output( + "{} doesn't exist, skip!".format(page.title())) + return None + except pywikibot.IsRedirectPage: + pywikibot.output('{} is a redirect, skip!'.format(page.title())) + return None + else: + text = old + return text + + +def put_text(page, new, summary, count, asynchronous=False): + """Save the new text.""" + page.text = new + try: + page.save(summary=summary, asynchronous=asynchronous, + minorEdit=page.namespace() != 3) + except pywikibot.EditConflict: + pywikibot.output('Edit conflict! skip!') + except pywikibot.ServerError: + if count <= config.max_retries: + pywikibot.output('Server Error! Wait..') + time.sleep(config.retry_wait) + return None + else: + raise pywikibot.ServerError( + 'Server Error! Maximum retries exceeded') + except pywikibot.SpamfilterError as e: + pywikibot.output( + 'Cannot change {} because of blacklist entry {}' + .format(page.title(), e.url)) + except pywikibot.LockedPage: + pywikibot.output('Skipping {} (locked page)'.format(page.title())) + except pywikibot.PageNotSaved as error: + pywikibot.output('Error putting page: {}'.format(error.args)) + else: + return True + return False + + def add_text(page, addText, summary=None, regexSkip=None, regexSkipUrl=None, always=False, up=False, putText=True, oldTextGiven=None, reorderEnabled=True, create=False): @@ -91,27 +142,13 @@ if not summary: summary = i18n.twtranslate(site, 'add_text-adding', {'adding': addText[:200]}) - - errorCount = 0 - if putText: pywikibot.output(u'Loading %s...' % page.title()) - if oldTextGiven is None: - try: - text = page.get() - except pywikibot.NoPage: - if create: - pywikibot.output(u"%s doesn't exist, creating it!" - % page.title()) - text = u'' - else: - pywikibot.output(u"%s doesn't exist, skip!" % page.title()) - return (False, False, always) - except pywikibot.IsRedirectPage: - pywikibot.output(u"%s is a redirect, skip!" % page.title()) - return (False, False, always) - else: - text = oldTextGiven + + text = get_text(page, oldTextGiven, create) + if text is None: + return (False, False, always) + # Understand if the bot has to skip the page or not # In this way you can use both -except and -excepturl if regexSkipUrl is not None: @@ -159,67 +196,43 @@ newtext += u"%s%s" % (config.line_separator, addText) else: newtext = addText + config.line_separator + text + if putText and text != newtext: pywikibot.output(color_format( '\n\n>>> {lightpurple}{0}{default} <<<', page.title())) pywikibot.showDiff(text, newtext) + # Let's put the changes. + error_count = 0 while True: # If someone load it as module, maybe it's not so useful to put the # text in the page - if putText: - if not always: - try: - choice = pywikibot.input_choice( - 'Do you want to accept these changes?', - [('Yes', 'y'), ('No', 'n'), ('All', 'a'), - ('open in Browser', 'b')], 'n') - except QuitKeyboardInterrupt: - sys.exit('User quit bot run.') - if choice == 'a': - always = True - elif choice == 'n': - return (False, False, always) - elif choice == 'b': - pywikibot.bot.open_webbrowser(page) - if always or choice == 'y': - try: - if always: - page.put(newtext, summary, - minorEdit=page.namespace() != 3) - else: - page.put_async(newtext, summary, - minorEdit=page.namespace() != 3) - except pywikibot.EditConflict: - pywikibot.output(u'Edit conflict! skip!') - return (False, False, always) - except pywikibot.ServerError: - errorCount += 1 - if errorCount < config.max_retries: - pywikibot.output(u'Server Error! Wait..') - time.sleep(config.retry_wait) - continue - else: - raise pywikibot.ServerError(u'Fifth Server Error!') - except pywikibot.SpamfilterError as e: - pywikibot.output( - u'Cannot change %s because of blacklist entry %s' - % (page.title(), e.url)) - return (False, False, always) - except pywikibot.LockedPage: - pywikibot.output(u'Skipping %s (locked page)' - % page.title()) - return (False, False, always) - except pywikibot.PageNotSaved as error: - pywikibot.output(u'Error putting page: %s' % error.args) - return (False, False, always) - else: - # Break only if the errors are one after the other... - errorCount = 0 - return (True, True, always) - else: + if not putText: return (text, newtext, always)

+ if not always: + try: + choice = pywikibot.input_choice( + 'Do you want to accept these changes?', + [('Yes', 'y'), ('No', 'n'), ('All', 'a'), + ('open in Browser', 'b')], 'n') + except QuitKeyboardInterrupt: + sys.exit('User quit bot run.') + + if choice == 'a': + always = True + elif choice == 'n': + return (False, False, always) + elif choice == 'b': + pywikibot.bot.open_webbrowser(page) + + if always or choice == 'y': + result = put_text(page, newtext, summary, error_count, + asynchronous=not always) + if result is not None: + return (result, result, always) + error_count += 1 +

def main(*args): """ diff --git a/tests/add_text_tests.py b/tests/add_text_tests.py index d696809..c96fa93 100644 --- a/tests/add_text_tests.py +++ b/tests/add_text_tests.py @@ -9,7 +9,7 @@

import pywikibot

-from scripts.add_text import add_text +from scripts.add_text import add_text, get_text

from tests.aspects import unittest, TestCase

@@ -23,11 +23,15 @@

dry = True

+ def setUp(self): + """Setup test.""" + super(TestAdding, self).setUp() + self.page = pywikibot.Page(self.site, 'foo') + def test_basic(self): """Test adding text.""" - page = pywikibot.Page(self.site, 'foo') (text, newtext, always) = add_text( - page, 'bar', putText=False, + self.page, 'bar', putText=False, oldTextGiven='foo\n{{linkfa}}') self.assertEqual( 'foo\n{{linkfa}}\nbar', @@ -35,14 +39,17 @@

def test_with_category(self): """Test adding text before categories.""" - page = pywikibot.Page(self.site, 'foo') (text, newtext, always) = add_text( - page, 'bar', putText=False, + self.page, 'bar', putText=False, oldTextGiven='foo\n[[Category:Foo]]') self.assertEqual( 'foo\nbar\n\n[[Category:Foo]]', newtext)

+ def test_get_text(self): + """Test get_text with given text.""" + self.assertEqual(get_text(self.page, 'foo', False), 'foo') +

if __name__ == '__main__': # pragma: no cover unittest.main()