tests: add tests.session create-session/destroy-session - #17469
Open
olivercalder wants to merge 4 commits into
Open
tests: add tests.session create-session/destroy-session#17469olivercalder wants to merge 4 commits into
olivercalder wants to merge 4 commits into
Conversation
Add new `tests.session` subcommands to create a systemd user session with a given class and type, and another subcommand to destroy a previously-created session. These are useful for tests which require a real systemd user session to be present. The existing `tests.session prepare` subcommand enables linger and starts the `default.target`, but it does not start a session. So the new subcommands should be used in conjunction with `prepare` when a test requires a real session. This is the case for any spread test which needs to run a user daemon, once the `ExecCondition=/usr/bin/snap routine user-service-precondition` is in place in their service files. Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
Add a comment explaining interactions between sessions created via `tests.session create-session` and `tests.session exec`. In short, these are separate sessions which should never interact. Commands run within `tests.session exec` run in their own background session, and will never be mistakenly identified with an existing session. And a non-background session has precedence over background sessions for the fallback session lookup of processes running outside any session, such as during systemd's `ExecCondition` evaluations. Add a comment explaining why `systemd-run` can be used on all systems. We don't use `--wait` or `--pipe`, which are the newer arguments not found on all systems. Remove the unnecessary `whitelist-environment` checks, as `pam_systemd` passes in the `XDG_SESSION_CLASS` and `XDG_SESSION_TYPE` variables for us automatically from the calling process. Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
olivercalder
requested review from
bboozzoo,
maykathm and
sergiocazzolato
and
a balanced review from Copilot
August 17, 2026 13:53
olivercalder
changed the title
Copilot stopped reviewing on behalf of
olivercalder due to an error
August 17, 2026 13:54
olivercalder
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #17469 +/- ##
==========================================
+ Coverage 78.75% 78.82% +0.07%
==========================================
Files 1404 1408 +4
Lines 197794 198088 +294
Branches 2502 2502
==========================================
+ Hits 155773 156148 +375
+ Misses 32651 32555 -96
- Partials 9370 9385 +15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Tue Aug 18 14:33:04 UTC 2026 Test Predictor AnalysisPreparing
Executing
Skipped tests from snapd-testing-skipIf you wish to have any of the below tests run in your PR, in your PR description, add 'unskip:' followed by a copy-and-pasted list of the below tests you wish to run (unskip plus test list must be valid yaml)
|
bboozzoo
reviewed
Aug 18, 2026
|
|
||
| systemctl stop "$unit_name" || true | ||
| systemctl reset-failed "$unit_name" || true | ||
| rm -f "$id_file" |
Contributor
There was a problem hiding this comment.
should we confirm somewhere that loginctl no longer lists the session with our ID?
Member
Author
There was a problem hiding this comment.
The spread test covers this. I've updated it to improve it more.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
olivercalder
requested review from
bboozzoo
and
a balanced review from Copilot
August 18, 2026 18:40
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
tests/lib/tools/tests.session:274
- This encoding is ambiguous: user
foo-barwithout a label and userfoowith labelbarboth map totests.session-persist-foo-bar.serviceand the same ID file. The second create can therefore report a false conflict, and destroy can stop another user's session. Use an unambiguous user key such as the numeric UID and apply the same encoding indestroy-session.
unit_name="tests.session-persist-$user$session_suffix.service"
id_file="/run/tests.session/$user$session_suffix.id"
tests/lib/tools/tests.session:321
echo "$XDG_SESSION_ID"writes a newline even when pam_systemd did not create a session, sotest -ssucceeds with an empty ID. When class and type are omitted, no laterloginctlcall detects this andcreate-sessionincorrectly reports success. Retry on a non-whitespace ID and clean up the transient unit if readiness times out.
retry -n 10 --wait 1 test -s "$id_file"
session_id="$(cat "$id_file")"
tests/lib/tools/suite/tests.session/task.yaml:107
- The new
-tbehavior is not exercised by this suite because every create call omits it, so regressions in propagating or verifyingXDG_SESSION_TYPEwould pass. Include a portable type in the initial creation; the implementation's own verification will then make this path observable.
tests.session create-session -u "$USER" -c user
|
|
||
| exit 0 | ||
| ;; | ||
| create-session) |
…prove testing Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
tests.session -u <username> preparefunction enables linger and startsdefault.target, but it doesn't actually create a systemd session. But we need a real session in some situations, as in #17380 where we need services to run a program inExecConditionwhich will look up the systemd session and decide whether to run based on its class.This PR adds two new commands:
These allow callers to create and destroy systemd user sessions with a given class and type. The optional
labelargument allows a label to be appended to the default session name, so it's possible to manage multiple sessions for the same user at the same time.The new subcommands should be used alongside
tests.session -u <username> prepareandtests.session -u <username> cleanupin tests which require a real systemd user session to be present.Cherry-picked from #17380
Tracked internally by: https://warthogs.atlassian.net/browse/SNAPDENG-37328