Merge lp:~robru/gwibber/baseplugin into lp:~barry/gwibber/py3

Proposed by Robert Bruce Park
Status: Merged
Merged at revision: 1403
Proposed branch: lp:~robru/gwibber/baseplugin
Merge into: lp:~barry/gwibber/py3
Diff against target: 74 lines (+44/-3)
2 files modified
gwibber/gwibber/plugins/base.py (+14/-0)
gwibber/gwibber/tests/test_plugins.py (+30/-3)
To merge this branch: bzr merge lp:~robru/gwibber/baseplugin
Reviewer Review Type Date Requested Status
Barry Warsaw Approve
Review via email: mp+121492@code.launchpad.net

Description of the change

Add BasePlugin._login method with test coverage. This method represents the common bits of the nearly verbatim Client._login method that existed in every plugin that we have.

To post a comment you must log in.
lp:~robru/gwibber/baseplugin updated
1402. By Barry Warsaw

[ Robert Bruce Park ]
Merged in larsu's libmessaging work and updated for py3 using 2to3.

1403. By Barry Warsaw

Merge robru's branch that refactors the _login() method into the BasePlugin.
I added a bunch of comments and did a few minor stylistic changes, along with
whitespace normalization.

Revision history for this message
Barry Warsaw (barry) wrote :

Approved and merged, with a few minor changes and a bunch of comments. Actually, I made one substantive change: I moved the return statement in BasePlugin._login() to inside the lock context manager. Putting it outside opens a small race condition I think, but it's too hard to test, so I just added a comment.

Revision history for this message
Barry Warsaw (barry) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'gwibber/gwibber/plugins/base.py'
2--- gwibber/gwibber/plugins/base.py 2012-08-22 21:06:25 +0000
3+++ gwibber/gwibber/plugins/base.py 2012-08-27 20:32:20 +0000
4@@ -25,3 +25,17 @@
5
6 def __call__(self, operation, **kws):
7 return getattr(self, operation)(**kws)
8+
9+ def _login(self):
10+ """Prevent redundant login attempts."""
11+ old_token = self.account.get('access_token')
12+ with self.account.login_lock:
13+ if self.account.get('access_token') == old_token:
14+ self._locked_login(old_token)
15+
16+ return self.account.get('access_token') != old_token
17+
18+ def _locked_login(self, old_token):
19+ """Login operation stub. This must be implemented by subclasses."""
20+ raise NotImplementedError
21+
22
23=== modified file 'gwibber/gwibber/tests/test_plugins.py'
24--- gwibber/gwibber/tests/test_plugins.py 2012-08-22 21:06:25 +0000
25+++ gwibber/gwibber/tests/test_plugins.py 2012-08-27 20:32:20 +0000
26@@ -21,18 +21,45 @@
27
28 import unittest
29
30+from threading import Lock
31+
32 from gwibber.plugins.base import BasePlugin
33
34
35+class FakeAccount(dict):
36+ def __init__(self):
37+ self.login_lock = Lock()
38+
39+
40+class MyPlugin(BasePlugin):
41+ """Simplest possible plugin to allow testing of BasePlugin."""
42+
43+ def noop(self, one=None, two=None):
44+ return '{}:{}'.format(one, two)
45+
46+ def _locked_login(self, old_token):
47+ self.account['access_token'] = 'fake_token'
48+
49+
50 class TestPlugins(unittest.TestCase):
51 """Test plugins."""
52
53 def test_basic_api(self):
54 # Plugins get instantiated with an account, and the instance gets
55 # called to perform an operation.
56- class MyPlugin(BasePlugin):
57- def noop(self, one=None, two=None):
58- return '{}:{}'.format(one, two)
59 fake_account = object()
60 my_plugin = MyPlugin(fake_account)
61 self.assertEqual(my_plugin('noop', one='one', two='two'), 'one:two')
62+
63+ def test_basic_login(self):
64+ my_plugin = MyPlugin(FakeAccount())
65+ self.assertTrue(my_plugin._login())
66+ self.assertFalse(my_plugin._login()) #Already logged in
67+
68+ def test_failing_login(self):
69+ class FailingPlugin(MyPlugin):
70+ def _locked_login(self, old_token):
71+ pass
72+ my_plugin = FailingPlugin(FakeAccount())
73+ self.assertFalse(my_plugin._login())
74+

Subscribers

People subscribed via source and target branches