Issue 1565001: test: Clean up site_login.py a bit.

Issue 1565001: test: Clean up site_login.py a bit. (Closed)

Created:
10 years, 9 months ago by Daniel Erat

Modified:
9 years, 7 months ago

Reviewers:
sosa

CC:
chromium-os-reviews_chromium.org, sosa+cc_chromium.org, seano, ericli, petkov+cc_chromium.org, Chris Masone, rginda

Visibility:
Public.

Description

test: Clean up site_login.py a bit. This has overlap with rginda's http://codereview.chromium.org/1534001, but there's one part that I prefer from my change: making all of these wait_ functions throw exceptions when they time out, which cuts down on a bunch of code that would otherwise need to be duplicated across all of our tests. I also ran into a bunch of races in UI tests when I reduced the polling timeout, so I've tried to fix those too. BUG=none TEST=ran the tests, both logged-in and logged-out

Patch Set 1 #

Patch Set 2 : update comments #

Patch Set 3 : work around some races #

Total comments: 8

Patch Set 4 : apply review feedback #

Total comments: 1

Patch Set 5 : get rid of a sleep() call in desktopui_KillRestart that leads to failure on usb #

Patch Set 6 : update one more call to attempt_logout() #

Created: 10 years, 9 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+206 lines, -224 lines)			Patch
M	client/bin/chromeos_constants.py	View		1 chunk	+3 lines, -0 lines	0 comments	Download
M	client/bin/site_login.py	View	1 2	3 chunks	+103 lines, -58 lines	0 comments	Download
M	client/bin/site_ui_test.py	View	1 2 3	1 chunk	+5 lines, -12 lines	0 comments	Download
A	client/bin/site_utils.py	View		1 chunk	+28 lines, -0 lines	0 comments	Download
M	client/site_tests/desktopui_ChromeFirstRender/desktopui_ChromeFirstRender.py	View		1 chunk	+5 lines, -6 lines	0 comments	Download
M	client/site_tests/desktopui_FailedLogin/desktopui_FailedLogin.py	View	1 2 3	1 chunk	+7 lines, -8 lines	0 comments	Download
M	client/site_tests/desktopui_IBusTest/desktopui_IBusTest.py	View	1 2 3	3 chunks	+13 lines, -23 lines	0 comments	Download
M	client/site_tests/desktopui_KillRestart/desktopui_KillRestart.py	View		2 chunks	+7 lines, -11 lines	0 comments	Download
M	client/site_tests/desktopui_ScreenSaverUnlock/desktopui_ScreenSaverUnlock.py	View	1 2 3	1 chunk	+6 lines, -16 lines	0 comments	Download
M	client/site_tests/desktopui_WindowManagerFocusNewWindows/desktopui_WindowManagerFocusNewWindows.py	View	1 2 3	2 chunks	+2 lines, -17 lines	0 comments	Download
M	client/site_tests/desktopui_WindowManagerHotkeys/desktopui_WindowManagerHotkeys.py	View	1 2 3	3 chunks	+8 lines, -43 lines	0 comments	Download
M	client/site_tests/logging_LogVolume/logging_LogVolume.py	View		1 chunk	+1 line, -2 lines	0 comments	Download
M	client/site_tests/login_ChromeProfileSanitary/login_ChromeProfileSanitary.py	View	1 2 3	2 chunks	+3 lines, -5 lines	0 comments	Download
M	client/site_tests/login_CryptohomeMounted/login_CryptohomeMounted.py	View	1 2 3	1 chunk	+8 lines, -9 lines	0 comments	Download
M	client/site_tests/login_CryptohomeUnmounted/login_CryptohomeUnmounted.py	View	1 2 3 4 5	2 chunks	+3 lines, -6 lines	0 comments	Download
M	client/site_tests/login_LogoutProcessCleanup/login_LogoutProcessCleanup.py	View		3 chunks	+3 lines, -6 lines	0 comments	Download
M	client/site_tests/platform_ProcessPrivileges/platform_ProcessPrivileges.py	View		1 chunk	+1 line, -2 lines	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

sosa

Hey can you leave some todo's where code should be refactored for UITest? http://codereview.chromium.org/1565001/diff/5001/6004 File ...

10 years, 9 months ago (2010-03-29 21:58:33 UTC) #2

Daniel Erat

http://codereview.chromium.org/1565001/diff/5001/6004 File client/bin/site_utils.py (right): http://codereview.chromium.org/1565001/diff/5001/6004#newcode27 client/bin/site_utils.py:27: 'Timed out waiting for condition') On 2010/03/29 21:58:33, sosa ...

10 years, 9 months ago (2010-03-29 23:07:12 UTC) #3

http://codereview.chromium.org/1565001/diff/5001/6004
File client/bin/site_utils.py (right):

http://codereview.chromium.org/1565001/diff/5001/6004#newcode27
client/bin/site_utils.py:27: 'Timed out waiting for condition')
On 2010/03/29 21:58:33, sosa wrote:
> What does this above exception mean?  Should it be if exception exception else
?

This is Python's ternary operator syntax: if the caller provided an exception,
we raise it; otherwise we generate our own.

http://codereview.chromium.org/1565001/diff/5001/6006
File client/site_tests/desktopui_FailedLogin/desktopui_FailedLogin.py (right):

http://codereview.chromium.org/1565001/diff/5001/6006#newcode21
client/site_tests/desktopui_FailedLogin/desktopui_FailedLogin.py:21:
site_login.wait_for_browser()
On 2010/03/29 21:58:33, sosa wrote:
> Why do you have wait_for_browser here but not in UITest?

Good point; this should no longer be needed.  Removed.

http://codereview.chromium.org/1565001/diff/5001/6007
File client/site_tests/desktopui_IBusTest/desktopui_IBusTest.py (right):

http://codereview.chromium.org/1565001/diff/5001/6007#newcode75
client/site_tests/desktopui_IBusTest/desktopui_IBusTest.py:75:
site_login.attempt_login(self, 'autox_script.json')
On 2010/03/29 21:58:33, sosa wrote:
> Care to refactor to UITest or leave a TODO in the test?

Sure, I've updated this test and the other ones (where it made sense) to use
UITest instead.

I'm still having trouble getting this particular one to pass, though -- it looks
like the ibusclient program is getting installed non-world-readable and owned by
my personal UID from the build chroot, so the test user isn't able to read it. 
Seems unrelated to my change, though.

http://codereview.chromium.org/1565001/diff/5001/6008
File
client/site_tests/desktopui_ScreenSaverUnlock/desktopui_ScreenSaverUnlock.py
(right):

http://codereview.chromium.org/1565001/diff/5001/6008#newcode21
client/site_tests/desktopui_ScreenSaverUnlock/desktopui_ScreenSaverUnlock.py:21:
site_login.attempt_logout()
On 2010/03/29 21:58:33, sosa wrote:
> Refactor or leave TODO

Done.

sosa

LGTM w/ other optional refactor. http://codereview.chromium.org/1565001/diff/14001/15013 File client/site_tests/login_CryptohomeMounted/login_CryptohomeMounted.py (right): http://codereview.chromium.org/1565001/diff/14001/15013#newcode21 client/site_tests/login_CryptohomeMounted/login_CryptohomeMounted.py:21: site_login.attempt_logout() Refactor?

10 years, 9 months ago (2010-03-29 23:20:55 UTC) #4

Daniel Erat

10 years, 9 months ago (2010-03-29 23:25:10 UTC) #5

On Mon, Mar 29, 2010 at 4:20 PM,  <sosa@chromium.org> wrote:
> LGTM w/ other optional refactor.
>
>
> http://codereview.chromium.org/1565001/diff/14001/15013
> File
> client/site_tests/login_CryptohomeMounted/login_CryptohomeMounted.py
> (right):
>
> http://codereview.chromium.org/1565001/diff/14001/15013#newcode21
> client/site_tests/login_CryptohomeMounted/login_CryptohomeMounted.py:21:
> site_login.attempt_logout()
> Refactor?

To use UITest?  I don't think it's possible for this one; note that it
gets run multiple times and needs to log in using different accounts,
hence the explicit logout at the beginning.

> http://codereview.chromium.org/1565001
>

Expand Messages | Collapse Messages