Merge lp:~jaboing/canonical-identity-provider/fix_edit_device_tests into lp:canonical-identity-provider/release

Proposed by Julien Funk
Status: Merged
Approved by: Natalia Bidart
Approved revision: no longer in the source branch.
Merged at revision: 968
Proposed branch: lp:~jaboing/canonical-identity-provider/fix_edit_device_tests
Merge into: lp:canonical-identity-provider/release
Diff against target: 303 lines (+114/-120)
7 files modified
acceptance/pages.py (+8/-0)
acceptance/tests/devices/__init__.py (+4/-0)
acceptance/tests/devices/test_device_login.py (+1/-1)
acceptance/tests/edit/__init__.py (+10/-0)
acceptance/tests/edit/testcases/__init__.py (+7/-0)
acceptance/tests/edit/testcases/test_device_preferences.py (+71/-106)
tools/pyflakes.txt (+13/-13)
To merge this branch: bzr merge lp:~jaboing/canonical-identity-provider/fix_edit_device_tests
Reviewer Review Type Date Requested Status
Natalia Bidart (community) Approve
Leo Arias (community) code review Approve
Review via email: mp+172082@code.launchpad.net

Commit message

Fixed the edit device preferences tests to be a test class and arranged the edit folder to accept both script and testcase tests.

To post a comment you must log in.
Revision history for this message
Leo Arias (elopio) wrote :

60 \ No newline at end of file

Please add the newline here ^^.

200 + always_radio = sst.actions.get_element(tag="input",
201 + name="twofactor_required", value="True")
202 + as_needed_radio = sst.actions.get_element(tag="input",
203 + name="twofactor_required", value="False")

Now that you are here, can you please add data-qa-ids to the radio buttons?

222 + one_time_pass = self.fixture.devices_fix.get_one_time_password(

I think this would be clearer if you define a get_one_time_password method in TwoFactorPageOpenedWithTwoFactorRequired that just calls the one in AuthenticationDevicesPageOpenedWithDevicesAdded.

I'm not sure, but I think we are missing a test that logs in with the preference saved "as needed".
We have the test for the preference saved as "always" on test_device_login.py. A comment mentioning that on top of the file would be nice.

Thanks you!

review: Needs Fixing (code review)
Revision history for this message
Leo Arias (elopio) :
review: Approve (code review)
Revision history for this message
Ubuntu One Auto Pilot (otto-pilot) wrote :
Download full text (45.9 KiB)

The attempt to merge lp:~jaboing/canonical-identity-provider/fix_edit_device_tests into lp:canonical-identity-provider failed. Below is the output from the failed tests.

Updating download cache at dir /mnt/tarmac/isd-download-cache
Using saved parent location: bzr+ssh://bazaar.launchpad.net/~canonical-isd-hackers/+junk/download-cache/
No revisions or tags to pull.
[localhost] local: which virtualenv
[localhost] local: /usr/bin/python /usr/bin/virtualenv --distribute --clear --system-site-packages .env
Not deleting .env/bin
New python executable in .env/bin/python
Installing distribute.............................................................................................................................................................................................done.
Installing pip...............done.
[localhost] local: dpkg -l libpq-dev 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l libxml2-dev 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l libxslt1-dev 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l memcached 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l postgresql-plpython-9.1 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l python-dev 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l python-m2crypto 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l swig 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l config-manager 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l python-egenix-mx-base-dev 2> /dev/null | grep '^ii' | wc -l
[localhost] local: /usr/lib/config-manager/cm.py update /tmp/tmpopCO84
[localhost] local: . /mnt/tarmac/cache/canonical-identity-provider/.env/bin/activate && make install PACKAGES="-r /mnt/tarmac/cache/canonical-identity-provider/requirements.txt"
pip install --find-links=file:///mnt/tarmac/isd-download-cache --no-index pip==dev
Ignoring indexes: http://pypi.python.org/simple/
Downloading/unpacking pip==dev
  Running setup.py egg_info for package pip

    warning: no files found matching '*.html' under directory 'docs'
    warning: no previously-included files matching '*.txt' found under directory 'docs/_build'
    no previously-included directories found matching 'docs/_build/_sources'
Installing collected packages: pip
  Found existing installation: pip 1.1
    Uninstalling pip:
      Successfully uninstalled pip
  Running setup.py install for pip

    warning: no files found matching '*.html' under directory 'docs'
    warning: no previously-included files matching '*.txt' found under directory 'docs/_build'
    no previously-included directories found matching 'docs/_build/_sources'
    Installing pip script to /mnt/tarmac/cache/canonical-identity-provider/.env/bin
    Installing pip-2.7 script to /mnt/tarmac/cache/canonical-identity-provider/.env/bin
Successfully installed pip
Cleaning up...
pip install --find-links=. --no-index -r /mnt/tarmac/cache/canonical-identity-provider/requirements.txt
Ignoring indexes: http://pypi.python.org/simple/
Downloading/unpacking bzr+http://bazaar.launchpad.net/~canonical-isd-hackers/u1-test-utils/trunk@81 (from -r /mnt/tarmac/cache/canonical-identity...

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

There are several pep8 and lint issues:

======================================================================
FAIL: test_pep8_conformance (identityprovider.tests.test_static_code_analysis.IdentityProviderPep8TestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/tarmac/cache/canonical-identity-provider/.env/local/lib/python2.7/site-packages/u1testutils/static/test_pep8_conformance.py", line 49, in test_pep8_conformance
    self.assertEqual(self.pep8style.options.report.total_errors, 0)
AssertionError: 2 != 0

======================================================================
FAIL: test_pyflakes_analysis (identityprovider.tests.test_static_code_analysis.PyFlakesTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/tarmac/cache/canonical-identity-provider/.env/local/lib/python2.7/site-packages/u1testutils/static/test_pyflakes_analysis.py", line 38, in test_pyflakes_analysis
    '\n'.join(unexpected_errors))
AssertionError: /mnt/tarmac/cache/canonical-identity-provider/./acceptance/tests/edit/scripts/illegitimate_passwords.py:31: undefined name 'password'
/mnt/tarmac/cache/canonical-identity-provider/./acceptance/tests/edit/scripts/illegitimate_passwords.py:31: undefined name 'passwordconfirm'
/mnt/tarmac/cache/canonical-identity-provider/./acceptance/tests/edit/scripts/illegitimate_passwords.py:33: undefined name 'password'
/mnt/tarmac/cache/canonical-identity-provider/./acceptance/tests/edit/scripts/illegitimate_passwords.py:34: undefined name 'passwordconfirm'
/mnt/tarmac/cache/canonical-identity-provider/./acceptance/tests/edit/scripts/illegitimate_passwords.py:38: undefined name 'error'
/mnt/tarmac/cache/canonical-identity-provider/./acceptance/tests/edit/scripts/illegitimate_passwords.py:41: undefined name 'error'
/mnt/tarmac/cache/canonical-identity-provider/./acceptance/tests/edit/scripts/illegitimate_passwords.py:44: undefined name 'error'
/mnt/tarmac/cache/canonical-identity-provider/./acceptance/tests/edit/scripts/illegitimate_passwords.py:47: undefined name 'error'
/mnt/tarmac/cache/canonical-identity-provider/./acceptance/tests/edit/scripts/valid_change_password.py:26: undefined name 'new_password'
/mnt/tarmac/cache/canonical-identity-provider/./acceptance/tests/edit/scripts/valid_change_password.py:28: undefined name 'new_password'
/mnt/tarmac/cache/canonical-identity-provider/./acceptance/tests/edit/scripts/valid_change_password.py:29: undefined name 'new_password'
/mnt/tarmac/cache/canonical-identity-provider/./acceptance/tests/edit/scripts/valid_change_password.py:35: undefined name 'new_password'
/mnt/tarmac/cache/canonical-identity-provider/./acceptance/tests/edit/scripts/change_password_bad.py:26: undefined name 'new_password'
/mnt/tarmac/cache/canonical-identity-provider/./acceptance/tests/edit/scripts/change_password_bad.py:27: undefined name 'new_password'

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

Note that the PEP8 errors weren't fixed yet:

/mnt/tarmac/cache/canonical-identity-provider/./acceptance/tests/edit/testcases/test_device_preferences.py:30:13: E128 continuation line under-indented for visual indent
            name="twofactor_required", value="True")
            ^
/mnt/tarmac/cache/canonical-identity-provider/./acceptance/tests/edit/testcases/test_device_preferences.py:32:13: E128 continuation line under-indented for visual indent
            name="twofactor_required", value="False")
            ^

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

Looks good!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'acceptance/pages.py'
2--- acceptance/pages.py 2013-07-07 01:10:57 +0000
3+++ acceptance/pages.py 2013-07-12 15:59:26 +0000
4@@ -176,6 +176,14 @@
5 sst.actions.click_button(sst.actions.get_element_by_css(
6 '*[data-qa-id="update_preferences"]'))
7
8+ def is_device_options_displayed(self):
9+ try:
10+ device_options = sst.actions.get_element_by_css(
11+ '*[data-qa-id="authentication_devices"]')
12+ return device_options.is_displayed()
13+ except AssertionError:
14+ return False
15+
16
17 class YourEmailAddresses(LoggedUserPage):
18
19
20=== modified file 'acceptance/tests/devices/__init__.py'
21--- acceptance/tests/devices/__init__.py 2013-07-06 07:40:10 +0000
22+++ acceptance/tests/devices/__init__.py 2013-07-12 15:59:26 +0000
23@@ -95,3 +95,7 @@
24 logout_landing_page = your_account_page.log_out()
25 login_page = logout_landing_page.click_log_in_or_create_account()
26 login_page.log_in_with_two_factor(self.user)
27+
28+ def get_one_time_password(self, device_number, password_index):
29+ return self.devices_fix.get_one_time_password(
30+ device_number, password_index)
31
32=== modified file 'acceptance/tests/devices/test_device_login.py'
33--- acceptance/tests/devices/test_device_login.py 2013-07-06 07:40:10 +0000
34+++ acceptance/tests/devices/test_device_login.py 2013-07-12 15:59:26 +0000
35@@ -18,7 +18,7 @@
36 self.page = pages.TwoFactorVerify()
37
38 def test_login_with_generic_device(self):
39- one_time_pass = self.fixture.devices_fix.get_one_time_password(
40+ one_time_pass = self.fixture.get_one_time_password(
41 device_number=0, password_index=1)
42 # Returns an YourAccount and automatically asserts it is open.
43 self.page.two_factor_authentication(one_time_pass)
44
45=== added file 'acceptance/tests/edit/__init__.py'
46--- acceptance/tests/edit/__init__.py 1970-01-01 00:00:00 +0000
47+++ acceptance/tests/edit/__init__.py 2013-07-12 15:59:26 +0000
48@@ -0,0 +1,10 @@
49+# Copyright 2013 Canonical Ltd.
50+# This software is licensed under the GNU Affero General Public License
51+# version 3 (see the file LICENSE).
52+
53+from sst import loader
54+
55+# TODO This is a temporary solution to combine script tests with test case
56+# classes. Once we are done with the refactor, we have to remove this extra
57+# package. -- elopio - 2013-05-20
58+discover = loader.discoverRegularTests
59
60=== added directory 'acceptance/tests/edit/scripts'
61=== renamed file 'acceptance/tests/edit/_authenticated_sites_u1.py' => 'acceptance/tests/edit/scripts/_authenticated_sites_u1.py'
62=== renamed file 'acceptance/tests/edit/authenticated_sites_askubuntu.py' => 'acceptance/tests/edit/scripts/authenticated_sites_askubuntu.py'
63=== renamed file 'acceptance/tests/edit/authenticated_sites_bitbucket.py' => 'acceptance/tests/edit/scripts/authenticated_sites_bitbucket.py'
64=== renamed file 'acceptance/tests/edit/change_password_bad.csv' => 'acceptance/tests/edit/scripts/change_password_bad.csv'
65=== renamed file 'acceptance/tests/edit/change_password_bad.py' => 'acceptance/tests/edit/scripts/change_password_bad.py'
66=== renamed file 'acceptance/tests/edit/edit_1_full_name.py' => 'acceptance/tests/edit/scripts/edit_1_full_name.py'
67=== renamed file 'acceptance/tests/edit/edit_2_preferred_email.py' => 'acceptance/tests/edit/scripts/edit_2_preferred_email.py'
68=== renamed file 'acceptance/tests/edit/edit_3_password.py' => 'acceptance/tests/edit/scripts/edit_3_password.py'
69=== renamed file 'acceptance/tests/edit/illegitimate_passwords.csv' => 'acceptance/tests/edit/scripts/illegitimate_passwords.csv'
70=== renamed file 'acceptance/tests/edit/illegitimate_passwords.py' => 'acceptance/tests/edit/scripts/illegitimate_passwords.py'
71=== renamed file 'acceptance/tests/edit/valid_change_password.csv' => 'acceptance/tests/edit/scripts/valid_change_password.csv'
72=== renamed file 'acceptance/tests/edit/valid_change_password.py' => 'acceptance/tests/edit/scripts/valid_change_password.py'
73=== added directory 'acceptance/tests/edit/testcases'
74=== added file 'acceptance/tests/edit/testcases/__init__.py'
75--- acceptance/tests/edit/testcases/__init__.py 1970-01-01 00:00:00 +0000
76+++ acceptance/tests/edit/testcases/__init__.py 2013-07-12 15:59:26 +0000
77@@ -0,0 +1,7 @@
78+from sst import loader
79+
80+
81+# TODO This is a temporary solution to combine script tests with test case
82+# classes. Once we are done with the refactor, we have to remove this extra
83+# package. -- elopio - 2013-05-20
84+discover = loader.discoverRegularTests
85
86=== renamed file 'acceptance/tests/edit/devices_prefs.py' => 'acceptance/tests/edit/testcases/test_device_preferences.py'
87--- acceptance/tests/edit/devices_prefs.py 2013-06-26 15:35:02 +0000
88+++ acceptance/tests/edit/testcases/test_device_preferences.py 2013-07-12 15:59:26 +0000
89@@ -1,106 +1,71 @@
90-# Test user two factor preferences
91-from urllib import quote
92-
93-from sst.actions import (
94- assert_checkbox_value,
95- assert_element,
96- assert_radio_value,
97- assert_url,
98- fails,
99- get_element,
100- go_to,
101-)
102-from u1testutils.sst import config
103-
104-from acceptance.devices import (
105- add_device,
106- authenticate,
107- delete_device,
108-)
109-
110-from acceptance import helpers, urls
111-
112-
113-def get_device_preferences_elements():
114- go_to(urls.EDIT)
115- always_radio = get_element(tag="input", name="twofactor_required",
116- value="True")
117- as_needed_radio = get_element(tag="input", name="twofactor_required",
118- value="False")
119- warn_backup_checkbox = get_element(tag="input",
120- name="warn_about_backup_device")
121- return always_radio, as_needed_radio, warn_backup_checkbox
122-
123-
124-def assert_device_preferences_not_displayed():
125- go_to(urls.EDIT)
126- fails(assert_element, tag="input", name="twofactor_required", value="True")
127- fails(assert_element, tag="input", name="twofactor_required",
128- value="False")
129- fails(assert_element, tag="input", name="warn_about_backup_device",
130- checked="checked")
131-
132-
133-def assert_device_preferences(
134- twofactor_always, twofactor_as_needed, warn_backup):
135- elements = get_device_preferences_elements()
136- always_radio, as_needed_radio, warn_backup_checkbox = elements
137- assert_radio_value(always_radio, twofactor_always)
138- assert_radio_value(as_needed_radio, twofactor_as_needed)
139- assert_checkbox_value(warn_backup_checkbox, warn_backup)
140-
141-
142-config.set_base_url_from_env()
143-
144-# Create account or login to test account
145-helpers.login_or_register_account(device_cleanup=True)
146-
147-# Test preferences form is present but disabled
148-assert_device_preferences_not_displayed()
149-
150-# Add an authentication device
151-add_device('prefs')
152-
153-# Test preferences form is now visible
154-# and the default is to not "always" require 2 factor authentication
155-assert_device_preferences(False, True, True)
156-
157-# Logout and back in so we are no longer 2f-authenticated
158-helpers.logout_and_in()
159-
160-# Change the preference to always require 2 factor authentication and save
161-helpers.set_twofactor_to_always_required()
162-assert_url('/two_factor_auth?next=%s' % quote(urls.EDIT))
163-
164-# 2 factor login
165-authenticate()
166-
167-# Check that "always" is now shown in the user preferences
168-assert_url(urls.EDIT)
169-
170-# and the default is to *always* require 2 factor authentication
171-assert_device_preferences(True, False, True)
172-
173-# reset always require to False
174-helpers.set_twofactor_to_required_as_needed()
175-assert_device_preferences(False, True, True)
176-
177-# clean up afterward, delete device
178-delete_device()
179-
180-# we're hiding preferences when no device is present
181-assert_device_preferences_not_displayed()
182-
183-# Check that the 2F is no longer required
184-# In response to defect #930379
185-helpers.logout_and_in()
186-go_to(urls.HOME)
187-assert_url(urls.HOME)
188-
189-# Add another device and
190-go_to(urls.DEVICES)
191-add_device('another name')
192-
193-# Logging out here tests that the devices clean up function works when
194-# a test ends up with two-factor required but logged out
195-helpers.logout_and_in()
196+# Copyright 2012, 2013 Canonical Ltd.
197+# This software is licensed under the GNU Affero General Public License
198+# version 3 (see the file LICENSE).
199+
200+from acceptance import base, pages
201+from acceptance.tests import devices
202+
203+import sst.actions
204+
205+
206+class TestDevicePreferences(base.SSTTestCaseWithLogIn):
207+
208+ def test_device_preferences_hidden(self):
209+ your_account_page = pages.YourAccount(open_page=True)
210+ self.assertFalse(your_account_page.is_device_options_displayed())
211+
212+ def test_device_preferences_visible_after_adding_device(self):
213+ self.useFixture(
214+ devices.AuthenticationDevicesPageOpenedWithDevicesAdded(
215+ ['Single device']))
216+ your_account_page = pages.YourAccount(open_page=True)
217+ self.assertTrue(your_account_page.is_device_options_displayed())
218+
219+ def test_two_factor_default_is_unrequired(self):
220+ self.useFixture(
221+ devices.AuthenticationDevicesPageOpenedWithDevicesAdded(
222+ ['Single device']))
223+ pages.YourAccount(open_page=True)
224+ always_radio = sst.actions.get_element(
225+ tag="input", name="twofactor_required", value="True")
226+ as_needed_radio = sst.actions.get_element(
227+ tag="input", name="twofactor_required", value="False")
228+ sst.actions.assert_radio_value(always_radio, False)
229+ sst.actions.assert_radio_value(as_needed_radio, True)
230+
231+ # In response to defect #923808
232+ def test_two_factor_preferences_not_displayed_after_device_deletion(self):
233+ self.useFixture(devices.DeleteDevicesPageOpened(
234+ 'Test device to delete'))
235+ self.page = pages.DeleteAuthenticationDevice()
236+ self.page.confirm_delete_device()
237+ your_account_page = pages.YourAccount(open_page=True)
238+ self.assertFalse(your_account_page.is_device_options_displayed())
239+
240+ # In response to defect #930379
241+ def test_two_factor_login_not_required_after_device_deletion(self):
242+ self.fixture = self.useFixture(
243+ devices.TwoFactorPageOpenedWithTwoFactorRequired(
244+ ['2F Required Generic Device'], self.user))
245+ self.page = pages.TwoFactorVerify()
246+ one_time_pass = self.fixture.get_one_time_password(
247+ device_number=0, password_index=1)
248+ self.page.two_factor_authentication(one_time_pass)
249+ your_auth_devices_page = pages.YourAuthenticationDevices(
250+ open_page=True)
251+ confirm_page = your_auth_devices_page.delete_authentication_device()
252+ confirm_page.confirm_delete_device()
253+ logout_landing_page = self.page.log_out()
254+ login_page = logout_landing_page.click_log_in_or_create_account()
255+ login_page.log_in_to_site_recognized(self.user)
256+ pages.YourAccount()
257+
258+ def test_two_factor_login_not_required_for_as_needed_setting(self):
259+ self.useFixture(
260+ devices.AuthenticationDevicesPageOpenedWithDevicesAdded(
261+ ['Single device']))
262+ self.page = pages.YourAuthenticationDevices()
263+ logout_landing_page = self.page.log_out()
264+ login_page = logout_landing_page.click_log_in_or_create_account()
265+ login_page.log_in_to_site_recognized(self.user)
266+ pages.YourAccount()
267
268=== modified file 'tools/pyflakes.txt'
269--- tools/pyflakes.txt 2013-06-04 19:53:34 +0000
270+++ tools/pyflakes.txt 2013-07-12 15:59:26 +0000
271@@ -32,19 +32,19 @@
272 acceptance/tests/emails/valid_address.py: undefined name 'address'
273 acceptance/tests/emails/valid_address.py: undefined name 'valid'
274 acceptance/tests/emails/valid_address.py: undefined name 'address'
275-acceptance/tests/edit/change_password_bad.py: undefined name 'new_password'
276-acceptance/tests/edit/change_password_bad.py: undefined name 'new_password'
277-acceptance/tests/edit/valid_change_password.py: undefined name 'new_password'
278-acceptance/tests/edit/valid_change_password.py: undefined name 'new_password'
279-acceptance/tests/edit/valid_change_password.py: undefined name 'new_password'
280-acceptance/tests/edit/illegitimate_passwords.py: undefined name 'password'
281-acceptance/tests/edit/illegitimate_passwords.py: undefined name 'passwordconfirm'
282-acceptance/tests/edit/illegitimate_passwords.py: undefined name 'password'
283-acceptance/tests/edit/illegitimate_passwords.py: undefined name 'passwordconfirm'
284-acceptance/tests/edit/illegitimate_passwords.py: undefined name 'error'
285-acceptance/tests/edit/illegitimate_passwords.py: undefined name 'error'
286-acceptance/tests/edit/illegitimate_passwords.py: undefined name 'error'
287-acceptance/tests/edit/illegitimate_passwords.py: undefined name 'error'
288+acceptance/tests/edit/scripts/change_password_bad.py: undefined name 'new_password'
289+acceptance/tests/edit/scripts/change_password_bad.py: undefined name 'new_password'
290+acceptance/tests/edit/scripts/valid_change_password.py: undefined name 'new_password'
291+acceptance/tests/edit/scripts/valid_change_password.py: undefined name 'new_password'
292+acceptance/tests/edit/scripts/valid_change_password.py: undefined name 'new_password'
293+acceptance/tests/edit/scripts/illegitimate_passwords.py: undefined name 'password'
294+acceptance/tests/edit/scripts/illegitimate_passwords.py: undefined name 'passwordconfirm'
295+acceptance/tests/edit/scripts/illegitimate_passwords.py: undefined name 'password'
296+acceptance/tests/edit/scripts/illegitimate_passwords.py: undefined name 'passwordconfirm'
297+acceptance/tests/edit/scripts/illegitimate_passwords.py: undefined name 'error'
298+acceptance/tests/edit/scripts/illegitimate_passwords.py: undefined name 'error'
299+acceptance/tests/edit/scripts/illegitimate_passwords.py: undefined name 'error'
300+acceptance/tests/edit/scripts/illegitimate_passwords.py: undefined name 'error'
301 acceptance/tests/root/links.py: undefined name 'link_text'
302 acceptance/tests/root/links.py: undefined name 'title'
303 acceptance/tests/root/login_requireds.py: undefined name 'email'