Merge into trunk : unlocker_fix_non_working

Status:	Merged
Approved by:	Albert Astals Cid on 2013-11-21
Approved revision:	524
Merged at revision:	544
Proposed branch:	lp:~om26er/unity8/unlocker_fix_non_working_code
Merge into:	lp:unity8
Diff against target:	11 lines (+1/-1) 1 file modified tests/autopilot/unity8/process_helpers.py (+1/-1)
To merge this branch:	bzr merge lp:~om26er/unity8/unlocker_fix_non_working_code
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot (community)	continuous-integration		Approve on 2013-11-21
Michał Sawicz			Needs Fixing on 2013-11-20
Christopher Lee (community)		2013-11-12	Approve on 2013-11-12
Michael Zanetti (community)		2013-11-12	Approve on 2013-11-12
Review via email: mp+194850@code.launchpad.net

Revision history for this message

Omer Akram (om26er) wrote on 2013-11-12:

#

I trying to get this change merged for the code to atleast work but I think this part of the code does not make sense.

If the unlocker is to be consumed by our apps their tests will fail because whenever we run a test suite of an app while the screen is unlocked the tests would fail or even if the tests are run with the screen locked after the first test runs every next test will fail since the screen will be unlocked. Instead of raising a warning I think it would make more sense to just use the python logger to log that the screen is already unlocked.

Revision history for this message

Michael Zanetti (mzanetti) wrote on 2013-11-12:

#

False is not defined

review: Needs Fixing

Revision history for this message

Michael Zanetti (mzanetti) wrote on 2013-11-12:

#

/me stupid... looks good

review: Approve

Revision history for this message

Christopher Lee (veebers) wrote on 2013-11-12:

#

Hi Omer,

Good catch with greeter not being None.

WRT raising an exception or logging; My thought process was that someone might want the greeter to be locked and thus would like to be alerted of that.

As it currently stands callers of unlock_unity() can wrap the call in a try..except.
I'm not married to this though and perhaps there is an argument that I'm trying to solve a problem that doesn't exist and that just logging makes the code cleaner and easier then we should change it to just log instead.

review: Approve

Revision history for this message

Omer Akram (om26er) wrote on 2013-11-12:

#

> Hi Omer,
>
> Good catch with greeter not being None.
>
> WRT raising an exception or logging; My thought process was that someone might
> want the greeter to be locked and thus would like to be alerted of that.

right, that might be really rare case but in the current form we have multiple problems i.e. only the first test passing and every other after that failing due to screen being unlocked.

>
> As it currently stands callers of unlock_unity() can wrap the call in a
> try..except.

This will result in each app doing exactly the same thing in its code, why not provide a reusable ensure_screen_unlock() ?

> I'm not married to this though and perhaps there is an argument that I'm
> trying to solve a problem that doesn't exist and that just logging makes the
> code cleaner and easier then we should change it to just log instead.

Lets move the conversation over to bug 1250458

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-17:

#

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-17:

#

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity8-ci/1629/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Francis Ginther (fginther) wrote on 2013-11-17:

#

Re-approving after resolving jenkins config error for generic-mediumtests-runner-mako job.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-17:

#

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity8-ci/1642/rebuild

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-18:

#

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-19:

#

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-19:

#

review: Needs Fixing (continuous-integration)

Revision history for this message

Omer Akram (om26er) wrote on 2013-11-19:

#

wow. Come on.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-19:

#

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-20:

#

review: Needs Fixing (continuous-integration)

Revision history for this message

Michał Sawicz (saviq) wrote on 2013-11-20:

#

Actually... I'm afraid there's a conflict here...

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-20:

#

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-21:

#

review: Needs Fixing (continuous-integration)

Revision history for this message

Christopher Lee (veebers) wrote on 2013-11-21:

#

The failure in the qmluitests is due to the testDashContents tests causing a segfault.
(You can see this by checking out the console log and searching for 'segmentation fault')

The segfault means that the newly created testDashContent.xml file is never populated with details. The jenkins job then sees this 0-length file and considers it indicative of a failed test and marks the job as UNSTABLE.

I'll just create a bug for this so we can keep track of it . . . here it is: https://bugs.launchpad.net/unity8/+bug/1253483

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-21:

#

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-21:

#

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/1686/rebuild

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-21:

#

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) on 2013-11-21:

#

review: Approve (continuous-integration)

1	=== modified file 'tests/autopilot/unity8/process_helpers.py'
2	--- tests/autopilot/unity8/process_helpers.py 2013-11-19 20:53:04 +0000
3	+++ tests/autopilot/unity8/process_helpers.py 2013-11-20 16:19:44 +0000
4	@@ -67,7 +67,7 @@
5	main_window = MainWindow(unity_proxy_obj)
6
7	greeter = main_window.get_greeter()
8	- if greeter is None:
9	+ if greeter.created == False:
10	raise RuntimeWarning("Greeter appears to be already unlocked.")
11	greeter.swipe()
12

Unity 8

Merge lp:~om26er/unity8/unlocker_fix_non_working_code into lp:unity8

Commit message

Description of the change

Preview Diff

Subscribers