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

Change subject: login.py: Unpack entry instead of overwriting itself
......................................................................


login.py: Unpack entry instead of overwriting itself

- Open the password file in a with block to make sure it is always closed
properly.
- login_tests.py was mocking `codecs.open` and the returned object was not a
 context manager; therefore did not work with our new `with` statement. Use
 `io.StringIO` to make it work again.
- Unpack `entry` into variables. This makes the code easier to follow and
 developers won't need to remember what each entry item means when examining
 the code.
- Fix a minor bug in file line numbers. Line numbers start from 1, not 0.
  Reverse the order of reading lines, so that we can break the password search
  loop as soon as we find a matching password entry. This way the priority of
  password entries won't change.

Change-Id: Ide50f0fd7d1baaa875907a17ed1e25801b6d472a
---
M pywikibot/login.py
M tests/login_tests.py
2 files changed, 20 insertions(+), 23 deletions(-)

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



diff --git a/pywikibot/login.py b/pywikibot/login.py
index fefbda7..dafcb84 100644
--- a/pywikibot/login.py
+++ b/pywikibot/login.py
@@ -248,8 +248,11 @@
         # We fix password file permission first.
         file_mode_checker(password_file, mode=config.private_files_permission)
 
-        password_f = codecs.open(password_file, encoding='utf-8')
-        for line_nr, line in enumerate(password_f):
+        with codecs.open(password_file, encoding='utf-8') as f:
+            lines = f.readlines()
+        line_nr = len(lines) + 1
+        for line in reversed(lines):
+            line_nr -= 1
             if not line.strip():
                 continue
             try:
@@ -265,23 +268,20 @@
                      'given)'.format(line_nr, entry), _PasswordFileWarning)
                 continue
 
-            # When the tuple is inverted the default family and code can be
-            # easily appended which makes the next condition easier as it does
-            # not need to know if it's using the default value or not.
-            entry = list(entry[::-1]) + [self.site.family.name,
-                                         self.site.code][len(entry) - 2:]
-
-            if (normalize_username(entry[1]) == self.username and
-                    entry[2] == self.site.family.name and
-                    entry[3] == self.site.code):
-                if isinstance(entry[0], basestring):
-                    self.password = entry[0]
-                elif isinstance(entry[0], BotPassword):
-                    self.password = entry[0].password
-                    self.login_name = entry[0].login_name(self.username)
+            code, family, username, password = (
+                self.site.code, self.site.family.name)[:4 - len(entry)] + entry
+            if (normalize_username(username) == self.username and
+                    family == self.site.family.name and
+                    code == self.site.code):
+                if isinstance(password, basestring):
+                    self.password = password
+                    break
+                elif isinstance(password, BotPassword):
+                    self.password = password.password
+                    self.login_name = password.login_name(self.username)
+                    break
                 else:
                     warn('Invalid password format', _PasswordFileWarning)
-        password_f.close()
 
     def login(self, retry=False):
         """
diff --git a/tests/login_tests.py b/tests/login_tests.py
index a1ee8ac..22c7c13 100644
--- a/tests/login_tests.py
+++ b/tests/login_tests.py
@@ -16,6 +16,7 @@
     import unittest.mock as mock
 except ImportError:
     import mock
+from io import StringIO
 
 from pywikibot.exceptions import NoUsername
 from pywikibot.login import LoginManager
@@ -93,10 +94,6 @@
         self.addCleanup(patcher.stop)
         return patcher.start()
 
-    def _file_lines(self, lines=[]):
-        for line in lines:
-            yield line
-
     def setUp(self):
         """Patch a variety of dependencies."""
         super(TestPasswordFile, self).setUp()
@@ -112,7 +109,7 @@
         self.chmod = self.patch("os.chmod")
 
         self.open = self.patch("codecs.open")
-        self.open.return_value = self._file_lines()
+        self.open.return_value = StringIO()
 
     def test_auto_chmod_OK(self):
         """Do not chmod files that have mode private_files_permission."""
@@ -132,7 +129,7 @@
         )
 
     def _test_pwfile(self, contents, password):
-        self.open.return_value = self._file_lines(contents.split("\n"))
+        self.open.return_value = StringIO(contents)
         obj = LoginManager()
         self.assertEqual(obj.password, password)
         return obj

-- 
To view, visit https://gerrit.wikimedia.org/r/373724
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ide50f0fd7d1baaa875907a17ed1e25801b6d472a
Gerrit-PatchSet: 12
Gerrit-Project: pywikibot/core
Gerrit-Branch: master
Gerrit-Owner: Dalba <[email protected]>
Gerrit-Reviewer: Dalba <[email protected]>
Gerrit-Reviewer: John Vandenberg <[email protected]>
Gerrit-Reviewer: Xqt <[email protected]>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to