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