jenkins-bot has submitted this change. ( 
https://gerrit.wikimedia.org/r/c/pywikibot/core/+/695548 )

Change subject: [IMPR] Refactor and test add_text argument parsing
......................................................................

[IMPR] Refactor and test add_text argument parsing

Splitting add_text's argument parsing into its own method with test coverage.
This is a pattern I loved within tor [1], and if approved I'll do the same for
other scripts as I go along.

The only functional changes to add_text are...

  * Rearranged -help arguments by prominence. Either -text or -textfile are
    mandatory so placing them first.

  * The '-newimages' argument was unimplemented. Removed it from our help.

  * Unrecognized arguments are now rejected rather than ignored.

  * Error if both -text and -textfile are used rather than ignoring the later.

[1] https://gitweb.torproject.org/stem.git/tree/stem/interpreter/arguments.py

Change-Id: I6b4ca274278c8a4488a1a82ec063181d668d4c36
---
M scripts/add_text.py
M tests/add_text_tests.py
2 files changed, 176 insertions(+), 68 deletions(-)

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



diff --git a/scripts/add_text.py b/scripts/add_text.py
index 1cfa980..b790d50 100755
--- a/scripts/add_text.py
+++ b/scripts/add_text.py
@@ -10,24 +10,22 @@

 Furthermore, the following command line parameters are supported:

--talkpage         Put the text onto the talk page instead
--talk
-
 -text             Define what text to add. "\n" are interpreted as newlines.

 -textfile         Define a texfile name which contains the text to add

 -summary          Define the summary to use

--excepturl        Use the html page as text where you want to see if there's
-                  the text, not the wiki-page.
-
--newimages        Add text in the new images
+-up               If used, put the text at the top of the page

 -always           If used, the bot won't ask if it should add the specified
                   text

--up               If used, put the text at the top of the page
+-talkpage         Put the text onto the talk page instead
+-talk
+
+-excepturl        Use the html page as text where you want to see if there's
+                  the text, not the wiki-page.

 -noreorder        Avoid reordering cats and interwiki

@@ -57,6 +55,7 @@
 # Distributed under the terms of the MIT license.
 #
 import codecs
+import collections
 import re
 import sys
 from typing import Optional, Union
@@ -78,6 +77,24 @@
 from pywikibot.tools import issue_deprecation_warning
 from pywikibot.tools.formatter import color_format

+DEFAULT_ARGS = {
+    'text': None,
+    'text_file': None,
+    'summary': None,
+    'up': False,
+    'always': False,
+    'talk_page': False,
+    'reorder': True,
+    'regex_skip_url': None,
+}
+
+ARG_PROMPT = {
+    '-text': 'What text do you want to add?',
+    '-textfile': 'Which text file do you want to append to the page?',
+    '-summary': 'What summary do you want to use?',
+    '-excepturl': 'What url pattern should we skip?',
+}
+

 docuReplacements = {'&params;': pagegenerators.parameterHelp}  # noqa: N816

@@ -274,76 +291,99 @@
         error_count += 1


-def main(*args: Tuple[str, ...]) -> None:
+def main(*argv: Tuple[str, ...]) -> None:
     """
     Process command line arguments and invoke bot.

     If args is an empty list, sys.argv is used.

-    @param args: Command line arguments
+    @param argv: Command line arguments
     """
-    # If none, the var is set only for check purpose.
-    summary = None
-    addText = None
-    regexSkipUrl = None
-    always = False
-    textfile = None
-    talkPage = False
-    reorderEnabled = True
+    generator_factory = pagegenerators.GeneratorFactory()

-    # Put the text above or below the text?
-    up = False
-
-    # Process global args and prepare generator args parser
-    local_args = pywikibot.handle_args(args)
-    genFactory = pagegenerators.GeneratorFactory()
-
-    # Loading the arguments
-    for arg in local_args:
-        option, sep, value = arg.partition(':')
-        if option == '-textfile':
-            textfile = value or pywikibot.input(
-                'Which textfile do you want to add?')
-        elif option == '-text':
-            addText = value or pywikibot.input('What text do you want to add?')
-        elif option == '-summary':
-            summary = value or pywikibot.input(
-                'What summary do you want to use?')
-        elif option == '-excepturl':
-            regexSkipUrl = value or pywikibot.input('What text should I skip?')
-        elif option == '-except':
-            new_arg = ''.join(['-grepnot', sep, value])
-            issue_deprecation_warning(arg, new_arg,
-                                      2, ArgumentDeprecationWarning,
-                                      since='20201224')
-            genFactory.handle_arg(new_arg)
-        elif option == '-up':
-            up = True
-        elif option == '-noreorder':
-            reorderEnabled = False
-        elif option == '-always':
-            always = True
-        elif option in ('-talk', '-talkpage'):
-            talkPage = True
-        else:
-            genFactory.handle_arg(arg)
-
-    if textfile and not addText:
-        with codecs.open(textfile, 'r', config.textfile_encoding) as f:
-            addText = f.read()
-
-    generator = genFactory.getCombinedGenerator()
-    additional_text = '' if addText else "The text to add wasn't given."
-    if pywikibot.bot.suggest_help(missing_generator=not generator,
-                                  additional_text=additional_text):
+    try:
+        args = parse(argv, generator_factory)
+    except ValueError as exc:
+        pywikibot.bot.suggest_help(additional_text=str(exc))
         return

-    if talkPage:
+    text = args.text
+
+    if args.text_file:
+        with codecs.open(args.text_file, 'r', config.textfile_encoding) as f:
+            text = f.read()
+
+    generator = generator_factory.getCombinedGenerator()
+
+    if pywikibot.bot.suggest_help(missing_generator=not generator):
+        return
+
+    if args.talk_page:
         generator = pagegenerators.PageWithTalkPageGenerator(generator, True)
+
     for page in generator:
-        add_text(page, addText, summary,
-                 regexSkipUrl=regexSkipUrl, always=always, up=up,
-                 reorderEnabled=reorderEnabled, create=talkPage)
+        add_text(page, text, args.summary,
+                 regexSkipUrl=args.regex_skip_url, always=args.always,
+                 up=args.up, reorderEnabled=args.reorder,
+                 create=args.talk_page)
+
+
+def parse(argv: Tuple[str, ...],
+          generator_factory: pagegenerators.GeneratorFactory
+          ) -> collections.namedtuple:
+    """
+    Parses our arguments and provide a named tuple with their values.
+
+    @param argv: input arguments to be parsed
+    @param generator_factory: factory that will determine the page to edit
+
+    @return: a namedtuple with our parsed arguments
+
+    @raise: ValueError if we receive invalid arguments
+    """
+    args = dict(DEFAULT_ARGS)
+    argv = pywikibot.handle_args(argv)
+    argv = generator_factory.handle_args(argv)
+
+    for arg in argv:
+        option, _, value = arg.partition(':')
+
+        if not value and option in ARG_PROMPT:
+            value = pywikibot.input(ARG_PROMPT[option])
+
+        if option == '-text':
+            args['text'] = value
+        elif option == '-textfile':
+            args['text_file'] = value
+        elif option == '-summary':
+            args['summary'] = value
+        elif option == '-up':
+            args['up'] = True
+        elif option == '-always':
+            args['always'] = True
+        elif option in ('-talk', '-talkpage'):
+            args['talk_page'] = True
+        elif option == '-noreorder':
+            args['reorder'] = False
+        elif option == '-except':
+            page_gen_arg = '-grepnot:{}'.format(value)
+            issue_deprecation_warning(arg, page_gen_arg,
+                                      2, ArgumentDeprecationWarning,
+                                      since='20201224')
+            generator_factory.handle_arg(page_gen_arg)
+        elif option == '-excepturl':
+            args['regex_skip_url'] = value
+        else:
+            raise ValueError("Argument '{}' is unrecognized".format(option))
+
+    if not args['text'] and not args['text_file']:
+        raise ValueError("Either the '-text' or '-textfile' is required")
+
+    if args['text'] and args['text_file']:
+        raise ValueError("'-text' and '-textfile' cannot both be used")
+
+    Args = collections.namedtuple('Args', args.keys())
+    return Args(**args)


 if __name__ == '__main__':
diff --git a/tests/add_text_tests.py b/tests/add_text_tests.py
index fca8331..30d987f 100644
--- a/tests/add_text_tests.py
+++ b/tests/add_text_tests.py
@@ -5,9 +5,14 @@
 # Distributed under the terms of the MIT license.
 #
 import unittest
+from unittest.mock import Mock, patch

 import pywikibot
-from scripts.add_text import add_text, get_text
+import pywikibot.pagegenerators
+
+from pywikibot.exceptions import ArgumentDeprecationWarning
+from pywikibot.tools import suppress_warnings
+from scripts.add_text import add_text, get_text, parse
 from tests.aspects import TestCase


@@ -24,6 +29,69 @@
         """Setup test."""
         super().setUp()
         self.page = pywikibot.Page(self.site, 'foo')
+        self.generator_factory = pywikibot.pagegenerators.GeneratorFactory()
+
+    @patch('pywikibot.handle_args', Mock(side_effect=lambda args: args))
+    def test_parse(self):
+        """Basic argument parsing."""
+        args = parse(['-text:"hello world"'], self.generator_factory)
+        self.assertEqual('"hello world"', args.text)
+        self.assertFalse(args.up)
+        self.assertTrue(args.reorder)
+
+        args = parse(['-text:hello', '-up', '-noreorder'],
+                     self.generator_factory)
+        self.assertEqual('hello', args.text)
+        self.assertTrue(args.up)
+        self.assertFalse(args.reorder)
+
+    @patch('pywikibot.handle_args', Mock(side_effect=lambda args: args))
+    def test_unrecognized_argument(self):
+        """Provide an argument that doesn't exist."""
+        expected_error = "Argument '-no_such_arg' is unrecognized"
+
+        for invalid_arg in ('-no_such_arg', '-no_such_arg:hello'):
+            with self.assertRaisesRegex(ValueError, expected_error):
+                parse([invalid_arg], self.generator_factory)
+
+    @patch('pywikibot.handle_args', Mock(side_effect=lambda args: args))
+    def test_neither_text_argument(self):
+        """Don't provide either -text or -textfile."""
+        expected_error = "Either the '-text' or '-textfile' is required"
+
+        with self.assertRaisesRegex(ValueError, expected_error):
+            parse(['-noreorder'], self.generator_factory)
+
+    @patch('pywikibot.handle_args', Mock(side_effect=lambda args: args))
+    def test_both_text_arguments(self):
+        """Provide both -text and -textfile."""
+        expected_error = "'-text' and '-textfile' cannot both be used"
+
+        with self.assertRaisesRegex(ValueError, expected_error):
+            parse(['-text:hello', '-textfile:/some/path'],
+                  self.generator_factory)
+
+    @patch('pywikibot.handle_args', Mock(side_effect=lambda args: args))
+    def test_except_argument(self):
+        """Check the deprecated -except argument."""
+        generator_factory = Mock()
+        generator_factory.handle_args.side_effect = lambda args: args
+
+        with suppress_warnings('-except:stuff is deprecated',
+                               ArgumentDeprecationWarning):
+            parse(['-text:hello', '-except:stuff'], generator_factory)
+
+        generator_factory.handle_arg.assert_called_with('-grepnot:stuff')
+
+    @patch('pywikibot.input')
+    @patch('pywikibot.handle_args', Mock(side_effect=lambda args: args))
+    def test_argument_prompt(self, input_mock):
+        """Reqest an argument that requres a prompt."""
+        input_mock.return_value = 'hello world'
+
+        args = parse(['-text'], self.generator_factory)
+        self.assertEqual('hello world', args.text)
+        input_mock.assert_called_with('What text do you want to add?')

     def test_basic(self):
         """Test adding text."""

--
To view, visit https://gerrit.wikimedia.org/r/c/pywikibot/core/+/695548
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.wikimedia.org/r/settings

Gerrit-Project: pywikibot/core
Gerrit-Branch: master
Gerrit-Change-Id: I6b4ca274278c8a4488a1a82ec063181d668d4c36
Gerrit-Change-Number: 695548
Gerrit-PatchSet: 4
Gerrit-Owner: Damian <[email protected]>
Gerrit-Reviewer: D3r1ck01 <[email protected]>
Gerrit-Reviewer: Xqt <[email protected]>
Gerrit-Reviewer: jenkins-bot
Gerrit-MessageType: merged
_______________________________________________
Pywikibot-commits mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to