After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 723698 - Implement smoketesting for multiple sessions
Implement smoketesting for multiple sessions
Status: RESOLVED FIXED
Product: gnome-continuous
Classification: Other
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: GNOME Continuous maintainer(s)
GNOME Continuous maintainer(s)
Depends on:
Blocks: 728059
 
 
Reported: 2014-02-05 17:12 UTC by Giovanni Campagna
Modified: 2014-04-25 13:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Implement smoketesting for multiple sessions (2.44 KB, patch)
2014-02-05 17:12 UTC, Giovanni Campagna
none Details | Review
Create user via accountservice dbus interface (1.93 KB, patch)
2014-04-18 13:52 UTC, Vadim Rutkovsky
reviewed Details | Review
Add a new 'smoketest-classic' task which starts gnome-classic (4.73 KB, patch)
2014-04-18 13:53 UTC, Vadim Rutkovsky
reviewed Details | Review
Wait longer during smoketests after gnome-session has staretd running (841 bytes, patch)
2014-04-18 13:54 UTC, Vadim Rutkovsky
none Details | Review
Wait for shell to startup (1018 bytes, patch)
2014-04-18 21:12 UTC, Vadim Rutkovsky
accepted-commit_after_freeze Details | Review

Description Giovanni Campagna 2014-02-05 17:12:19 UTC
Not tested, and I'm afraid I can't setup a continuous testing VM.
But it should work in theory.
Comment 1 Giovanni Campagna 2014-02-05 17:12:21 UTC
Created attachment 268189 [details] [review]
Implement smoketesting for multiple sessions

Write an AccountsService file before smoketesting to force a
specific session, so that we can run smoketests on gnome core
and gnome classic.
Comment 2 Vadim Rutkovsky 2014-04-18 13:49:54 UTC
Review of attachment 268189 [details] [review]:

This is not gonna work - in _prepareDisks user is not yet created
Comment 3 Vadim Rutkovsky 2014-04-18 13:52:32 UTC
Created attachment 274669 [details] [review]
Create user via accountservice dbus interface

This patch is assuming that created user will have uid 1000, but I'm not sure how to read user's UID correctly
Comment 4 Vadim Rutkovsky 2014-04-18 13:53:14 UTC
Created attachment 274670 [details] [review]
Add a new 'smoketest-classic' task which starts gnome-classic
Comment 5 Vadim Rutkovsky 2014-04-18 13:54:24 UTC
Created attachment 274671 [details] [review]
Wait longer during smoketests after gnome-session has staretd running

Before bug 728449 is solved, lets just wait longer to get a nice screenshot
Comment 6 Giovanni Campagna 2014-04-18 14:13:41 UTC
(In reply to comment #2)
> Review of attachment 268189 [details] [review]:
> 
> This is not gonna work - in _prepareDisks user is not yet created

Why is that a problem? We're not running the image until later, and we don't need the UID, just the name.
Comment 7 Vadim Rutkovsky 2014-04-18 21:04:12 UTC
(In reply to comment #6)
> (In reply to comment #2)
> > Review of attachment 268189 [details] [review] [details]:
> > 
> > This is not gonna work - in _prepareDisks user is not yet created
> 
> Why is that a problem? We're not running the image until later, and we don't
> need the UID, just the name.

Tried this locally - and it didn't work. Probably accountservice overwrites the file later, as the user is being created later (in testbase.execute) and via useradd utility, but that's just a guess
Comment 8 Vadim Rutkovsky 2014-04-18 21:12:21 UTC
Created attachment 274717 [details] [review]
Wait for shell to startup
Comment 10 Colin Walters 2014-04-25 13:12:57 UTC
Review of attachment 274669 [details] [review]:

I'm *definitely* a fan of using the accountsservice API rather than useradd directly, as we end up testing accountsservice too.

But Ray should probably review this one...it might be worth creating accountsservice commandline utilities.

::: src/js/libqa.js
@@ +236,3 @@
     let addUserService = '[Unit]\n\
 Description=Add user %s\n\
+After=accountsservice.service\n\

Don't need this, dbus activation will start accountsservice on demand for us.
Comment 11 Colin Walters 2014-04-25 13:19:43 UTC
Review of attachment 274670 [details] [review]:

::: src/js/libqa.js
@@ +235,3 @@
     }
+    if (params.session != null) {
+        setSessionCommand = Format.vprintf(commandTemplate, ['/User1000', 'User.SetXSession', 'string:' + params.session])

Jasper did this in a different way by writing to /var/lib/AccountsService directly which is ugly, but works.  I like this approach more - do you want to rebase this patch on master and drop Jasper's code to write to /var/lib?

::: src/js/tasks/task-smoketest.js
@@ +53,3 @@
+
+
+const TaskSmoketestClassic = new Lang.Class({

Or we could apply just this bit on master now.
Comment 12 Colin Walters 2014-04-25 13:20:54 UTC
Review of attachment 274717 [details] [review]:

This looks good to me, once the shell patch has landed.