Merge lp:~unity-team/unity8/test-early-disable into lp:unity8

Proposed by Michał Sawicz
Status: Merged
Approved by: Michał Sawicz
Approved revision: 1487
Merged at revision: 1539
Proposed branch: lp:~unity-team/unity8/test-early-disable
Merge into: lp:unity8
Prerequisite: lp:~mterry/unity8/cleanup-greeter-dbus-test
Diff against target: 111 lines (+44/-1)
4 files modified
tests/mocks/LightDM/Greeter.cpp (+6/-0)
tests/mocks/LightDM/Greeter.h (+3/-0)
tests/plugins/LightDM/dbus.cpp (+4/-1)
tests/qmltests/tst_ShellWithPin.qml (+31/-0)
To merge this branch: bzr merge lp:~unity-team/unity8/test-early-disable
Reviewer Review Type Date Requested Status
Michał Sawicz Approve
PS Jenkins bot continuous-integration Pending
Albert Astals Cid Pending
Review via email: mp+245855@code.launchpad.net

This proposal supersedes a proposal from 2014-12-11.

Commit message

Add a test to make sure the shell always starts disabled until it is enabled by a complete PAM interaction.

Description of the change

Add a test to make sure the shell always starts disabled until it is enabled by a complete PAM interaction.

== Checklist ==

 * Are there any related MPs required for this MP to build/function as expected? Please list.
 No

 * Did you perform an exploratory manual test run of your code change and any related functionality?
 Yes

 * Did you make sure that your branch does not contain spurious tags?
 Yes, for realz

 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
 NA

 * If you changed the UI, has there been a design review?
 NA

To post a comment you must log in.
Revision history for this message
Albert Astals Cid (aacid) wrote : Posted in a previous version of this proposal

 * Did you perform an exploratory manual test run of the code change and any related functionality?
Yes

 * Did CI run pass?
Because CI is dead Jim :/

 * Did you make sure that the branch does not contain spurious tags?
Yes

review: Approve
Revision history for this message
Michał Sawicz (saviq) wrote : Posted in a previous version of this proposal

This causes a test failure:

make testLightDMDBus

********* Start testing of GreeterDBusTest *********
Config: Using QtTest library 5.3.2, Qt 5.3.2
PASS : GreeterDBusTest::initTestCase()
PASS : GreeterDBusTest::testGetActiveEntry()
PASS : GreeterDBusTest::testSetActiveEntry()
PASS : GreeterDBusTest::testEntrySelectedSignal()
PASS : GreeterDBusTest::testActiveEntryGet()
PASS : GreeterDBusTest::testActiveEntrySet()
PASS : GreeterDBusTest::testActiveEntryChanged()
FAIL! : GreeterDBusTest::testEntryIsLockedGet() '!dbusList->property("EntryIsLocked").toBool()' returned FALSE. ()
   Loc: [../tests/plugins/LightDM/dbus.cpp(152)]
FAIL! : GreeterDBusTest::testEntryIsLockedChanged() 'arguments.at(1).toMap().contains("EntryIsLocked")' returned FALSE. ()
   Loc: [../tests/plugins/LightDM/dbus.cpp(167)]
PASS : GreeterDBusTest::testIsActive()
PASS : GreeterDBusTest::testShowGreeter()
PASS : GreeterDBusTest::cleanupTestCase()
Totals: 10 passed, 2 failed, 0 skipped
********* Finished testing of GreeterDBusTest *********

review: Needs Fixing
Revision history for this message
Michał Sawicz (saviq) wrote : Posted in a previous version of this proposal

And on that note, the above test does not leave a .xml file around... And is named GreeterDBusTest, where it should be LightDMTest?

Please rebase this on a branch fixing the above.

Revision history for this message
Michael Terry (mterry) wrote : Posted in a previous version of this proposal

Fixed test. I didn't get that far when running qmluitests myself because of the tstDash failure. Should have worked around it rather than give up. :-/

Revision history for this message
Michael Terry (mterry) wrote : Posted in a previous version of this proposal

And rebased on a cleanup branch for the GreeterDBus test as requested.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:1486
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~mterry/unity8/test-early-disable/+merge/244461/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/unity8-ci/5029/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/194
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/194

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

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Albert Astals Cid (aacid) wrote : Posted in a previous version of this proposal

 * Did you perform an exploratory manual test run of the code change and any related functionality?
Yes, tests look good

 * Did CI run pass?
Half-ish, CI is half alive only

 * Did you make sure that the branch does not contain spurious tags?
Yes

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
Michał Sawicz (saviq) wrote :

Approving per the superseded merge, this is just a remerge.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tests/mocks/LightDM/Greeter.cpp'
2--- tests/mocks/LightDM/Greeter.cpp 2014-07-31 15:59:04 +0000
3+++ tests/mocks/LightDM/Greeter.cpp 2015-01-08 13:37:03 +0000
4@@ -129,6 +129,12 @@
5 d->authenticated = false;
6 d->authenticationUser = username;
7 d->twoFactorDone = false;
8+ QTimer::singleShot(0, this, SLOT(delayedAuthentication()));
9+}
10+
11+void Greeter::delayedAuthentication()
12+{
13+ Q_D(Greeter);
14 d->handleAuthenticate();
15 }
16
17
18=== modified file 'tests/mocks/LightDM/Greeter.h'
19--- tests/mocks/LightDM/Greeter.h 2014-07-31 15:59:04 +0000
20+++ tests/mocks/LightDM/Greeter.h 2015-01-08 13:37:03 +0000
21@@ -99,6 +99,9 @@
22 protected:
23 void sendAuthenticationComplete();
24
25+private Q_SLOTS:
26+ void delayedAuthentication();
27+
28 private:
29 GreeterPrivate *d_ptr;
30 Q_DECLARE_PRIVATE(Greeter)
31
32=== modified file 'tests/plugins/LightDM/dbus.cpp'
33--- tests/plugins/LightDM/dbus.cpp 2015-01-08 13:37:02 +0000
34+++ tests/plugins/LightDM/dbus.cpp 2015-01-08 13:37:03 +0000
35@@ -16,6 +16,7 @@
36
37 #include "Greeter.h"
38
39+#include <QCoreApplication>
40 #include <QDBusInterface>
41 #include <QDBusReply>
42 #include <QSignalSpy>
43@@ -149,9 +150,11 @@
44 QVERIFY(dbusList->property("EntryIsLocked").toBool());
45
46 greeter->authenticate("no-password");
47+ QCoreApplication::processEvents(); // wait for auth to finish
48 QVERIFY(!dbusList->property("EntryIsLocked").toBool());
49
50 greeter->authenticate("has-password");
51+ QCoreApplication::processEvents(); // wait for auth to finish
52 QVERIFY(dbusList->property("EntryIsLocked").toBool());
53 }
54
55@@ -168,7 +171,7 @@
56 }
57 QCOMPARE(spy.count(), 2);
58
59- QList<QVariant> arguments = spy.takeFirst();
60+ QList<QVariant> arguments = spy.takeLast();
61 QVERIFY(arguments.at(0).toString() == "com.canonical.UnityGreeter.List");
62 QVERIFY(arguments.at(1).toMap().contains("EntryIsLocked"));
63 QVERIFY(arguments.at(1).toMap()["EntryIsLocked"] == false);
64
65=== modified file 'tests/qmltests/tst_ShellWithPin.qml'
66--- tests/qmltests/tst_ShellWithPin.qml 2014-11-24 14:03:06 +0000
67+++ tests/qmltests/tst_ShellWithPin.qml 2015-01-08 13:37:03 +0000
68@@ -116,6 +116,12 @@
69 signalName: "resettingDevice"
70 }
71
72+ SignalSpy {
73+ id: promptSpy
74+ target: LightDM.Greeter
75+ signalName: "showPrompt"
76+ }
77+
78 Telephony.CallEntry {
79 id: phoneCall
80 phoneNumber: "+447812221111"
81@@ -472,5 +478,30 @@
82
83 }
84
85+ /* We had a bug (1395075) where if a user kept swiping as the greeter
86+ loaded, they would be able to get into the session before the
87+ lockscreen appeared. Make sure that doesn't happen. */
88+ function test_earlyDisable() {
89+ // Kill current shell
90+ shellLoader.itemDestroyed = false;
91+ shellLoader.active = false;
92+ tryCompare(shellLoader, "itemDestroyed", true);
93+ LightDM.Greeter.authenticate(""); // reset greeter
94+
95+ // Create new shell
96+ promptSpy.clear();
97+ shellLoader.active = true;
98+ tryCompareFunction(function() {return shell !== null}, true);
99+
100+ // Confirm that we start disabled
101+ compare(promptSpy.count, 0);
102+ verify(!shell.enabled);
103+
104+ // And that we only become enabled once the lockscreen is up
105+ tryCompare(shell, "enabled", true);
106+ verify(promptSpy.count > 0);
107+ var lockscreen = findChild(shell, "lockscreen");
108+ verify(lockscreen.shown);
109+ }
110 }
111 }

Subscribers

People subscribed via source and target branches