tests: add audio-record prompting integration tests - #17015
Merged
olivercalder merged 7 commits intoMay 6, 2026
Merged
Conversation
olivercalder
requested review from
andrewphelpsj,
Copilot,
natibek and
sergiocazzolato
May 1, 2026 22:00
Contributor
There was a problem hiding this comment.
Pull request overview
Adds new spread variants to the apparmor-prompting-integration-tests suite to exercise AppArmor prompting end-to-end for the audio-record interface, including installing a helper snap and cleaning up prompting state between runs.
Changes:
- Add
audio-recordspread variants and install/connect aprompt-requesterhelper snap for those variants. - Add scripted prompting-client JSON + shell drivers for
audio-recordsingle and timespan scenarios. - Ensure prompting request/rule state is removed on restore to avoid cross-test interference.
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/main/apparmor-prompting-integration-tests/task.yaml | Adds new audio-record variants, installs helper snap for those variants, and cleans prompting state on restore. |
| tests/main/apparmor-prompting-integration-tests/prompt-requester/meta/snap.yaml | Defines a helper snap used to create a process/cgroup context for ask requests. |
| tests/main/apparmor-prompting-integration-tests/prompt-requester/bin/wait-for | Helper app that stays running until a “finish” file appears (used to keep a stable PID/cgroup). |
| tests/main/apparmor-prompting-integration-tests/prompt-requester/bin/cat | Simple wrapper around /bin/cat (part of helper snap). |
| tests/main/apparmor-prompting-integration-tests/audio_record_single.sh | Drives ask requests for audio-record and validates outcomes across multiple prompts. |
| tests/main/apparmor-prompting-integration-tests/audio_record_single.json | Scripted prompting-client prompt filters + replies for the single-lifespan flow. |
| tests/main/apparmor-prompting-integration-tests/audio_record_timespan_allow.sh | Drives ask requests and validates allow-timespan then deny after expiry. |
| tests/main/apparmor-prompting-integration-tests/audio_record_timespan_allow.json | Scripted replies for allow-timespan then deny. |
| tests/main/apparmor-prompting-integration-tests/audio_record_timespan_deny.sh | Drives ask requests and validates deny-timespan then allow after expiry. |
| tests/main/apparmor-prompting-integration-tests/audio_record_timespan_deny.json | Scripted replies for deny-timespan then allow. |
|
Wed May 6 15:08:19 UTC 2026 Failures:Preparing:
Executing:
Restoring:
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 (without variants) of the below tests you wish to run (unskip plus test list must be valid yaml)
|
natibek
reviewed
May 4, 2026
natibek
left a comment
natibek
left a comment
Contributor
There was a problem hiding this comment.
Thanks! Just one question.
| MATCH '"outcome": "allow"' < "$RESULT" | ||
|
|
||
| # Trigger a fourth request | ||
| ASK_BODY="{\"action\": \"ask\", \"interface\": \"audio-record\", \"pid\": $WAITER_SNAP_PID}" |
Contributor
There was a problem hiding this comment.
Is there a concern that this fourth request is triggered close to the 10second duration of the initial response?
Member
Author
There was a problem hiding this comment.
I don't think so, these requests should be basically instantaneous.
olivercalder
force-pushed
the
prompting-add-audio-record-integration-tests
branch
from
May 4, 2026 20:26
5d71300 to
2dfa5ba
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #17015 +/- ##
==========================================
+ Coverage 79.07% 79.13% +0.05%
==========================================
Files 1378 1367 -11
Lines 191273 191111 -162
Branches 2465 2465
==========================================
- Hits 151247 151232 -15
+ Misses 30920 30779 -141
+ Partials 9106 9100 -6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| echo "Run the test script as the test user" | ||
| if ! tests.session -u test exec sh -x "${TEST_DIR}/${VARIANT}.sh" "$TEST_DIR" "$TIMEOUT"; then | ||
| if ! tests.session -u test exec sh -xe "${TEST_DIR}/${VARIANT}.sh" "$TEST_DIR" "$TIMEOUT"; then |
Member
Author
There was a problem hiding this comment.
Not having -e confused me a lot, so it makes more sense to keep -e and instead specially handle the places in which we expect failures to occur.
|
|
||
| for name in test1.txt test2.txt test3.txt test4.txt ; do | ||
| TEST_OUTPUT="$(cat "${WRITABLE}/${name}")" | ||
| TEST_OUTPUT="$(cat "${WRITABLE}/${name}" || true)" |
Member
Author
There was a problem hiding this comment.
That's why we check $TEST_OUTPUT on the next line
Comment on lines
+18
to
+24
| for i in $(seq "$TIMEOUT") ; do | ||
| if ! pgrep -af "prompting-client.scripted.*${TEST_DIR}" ; then | ||
| break | ||
| fi | ||
| sleep 1 | ||
| done | ||
| if pgrep -af "prompting-client.scripted.*${TEST_DIR}" ; then |
Member
Author
There was a problem hiding this comment.
This was deliberate, if this is hanging I want to see what's running.
Comment on lines
51
to
+53
| # expect any rules with duplicate path patterns. | ||
| snap debug api /v2/interfaces/requests/rules | jq '."result".[]."constraints"."path-pattern"' | grep "${TEST_DIR}" | uniq -c | MATCH '^[[:space:]]*1' | ||
| snap debug api /v2/interfaces/requests/rules | jq '."result".[]."constraints"."path-pattern"' | grep "${TEST_DIR}" | uniq -c | NOMATCH '^[[:space:]]*2' | ||
| snap debug api /v2/interfaces/requests/rules | jq '."result".[]."constraints"."path-pattern"' | grep "${TEST_DIR}" | uniq -c | grep '^[[:space:]]*1' | ||
| ! snap debug api /v2/interfaces/requests/rules | jq '."result".[]."constraints"."path-pattern"' | grep "${TEST_DIR}" | uniq -c | grep -q '^[[:space:]]*[^1[[:space:]]]' |
Comment on lines
49
to
+52
| # Rules with identical path patterns are merged, so we don't expect any rules | ||
| # with duplicate path patterns. | ||
| snap debug api /v2/interfaces/requests/rules | jq '."result".[]."constraints"."path-pattern"' | grep "${TEST_DIR}" | uniq -c | MATCH '^[[:space:]]*1' | ||
| snap debug api /v2/interfaces/requests/rules | jq '."result".[]."constraints"."path-pattern"' | grep "${TEST_DIR}" | uniq -c | NOMATCH '^[[:space:]]*2' | ||
| snap debug api /v2/interfaces/requests/rules | jq '."result".[]."constraints"."path-pattern"' | grep "${TEST_DIR}" | uniq -c | grep '^[[:space:]]*1' | ||
| ! snap debug api /v2/interfaces/requests/rules | jq '."result".[]."constraints"."path-pattern"' | grep "${TEST_DIR}" | uniq -c | grep -q '^[[:space:]]*[^1[[:space:]]]' |
Comment on lines
+20
to
+24
| # Start the snap running in the background so "ask" can use its PID to look up | ||
| # its cgroup, and from that derive the snap name. | ||
| prompt-requester.wait-for "$TARGET_FILE" & | ||
| WAITER_SHELL_PID="$!" | ||
|
|
Comment on lines
+20
to
+24
| # Start the snap running in the background so "ask" can use its PID to look up | ||
| # its cgroup, and from that derive the snap name. | ||
| prompt-requester.wait-for "$TARGET_FILE" & | ||
| WAITER_SHELL_PID="$!" | ||
|
|
Comment on lines
+20
to
+24
| # Start the snap running in the background so "ask" can use its PID to look up | ||
| # its cgroup, and from that derive the snap name. | ||
| prompt-requester.wait-for "$TARGET_FILE" & | ||
| WAITER_SHELL_PID="$!" | ||
|
|
natibek
requested changes
May 5, 2026
| # expect any rules with duplicate path patterns. | ||
| snap debug api /v2/interfaces/requests/rules | jq '."result".[]."constraints"."path-pattern"' | grep "${TEST_DIR}" | uniq -c | MATCH '^[[:space:]]*1' | ||
| snap debug api /v2/interfaces/requests/rules | jq '."result".[]."constraints"."path-pattern"' | grep "${TEST_DIR}" | uniq -c | NOMATCH '^[[:space:]]*2' | ||
| snap debug api /v2/interfaces/requests/rules | jq '."result".[]."constraints"."path-pattern"' | grep "${TEST_DIR}" | uniq -c | grep '^[[:space:]]*1' |
Contributor
There was a problem hiding this comment.
I think this pattern needs to be something like ^[[:space:]]*1[[:space:]]*$ to avoid matching any number starting with the digit 1.
Contributor
There was a problem hiding this comment.
Though, this is a bit extreme I realize since we only send the request twice.
Member
Author
There was a problem hiding this comment.
That was my rationale too... I'll slot in a fix if there are any other things to change, but otherwise I don't think it's necessary. The regex would have to be this I think: ^[[:space:]]*1[[:space:]] since after the number and a space there will be other text.
andrewphelpsj
approved these changes
May 5, 2026
Comment on lines
+24
to
+33
| for i in $(seq "$TIMEOUT") ; do | ||
| if ! pgrep -af "prompting-client.scripted.*${TEST_DIR}" ; then | ||
| break | ||
| fi | ||
| sleep 1 | ||
| done | ||
| if pgrep -af "prompting-client.scripted.*${TEST_DIR}" ; then | ||
| echo "prompting-client.scripted still running" | ||
| exit 1 | ||
| fi |
Member
There was a problem hiding this comment.
You are really doing this construct a lot, maybe worth abstracting one day, but not high priority.
Member
Author
There was a problem hiding this comment.
Yeah I had an abstraction via timeout and while loop but then the timeout and while commands were being caught in the pgrep lookup, causing it to never see itself as done. And pgrep -f doesn't work with binary names longer than 15 characters so I gave up and did the most direct thing. If you have ideas though I'm all ears.
olivercalder
added
the
Auto rerun spread
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
olivercalder
force-pushed
the
prompting-add-audio-record-integration-tests
branch
from
May 6, 2026 13:23
bcf60fb to
9d51f3d
Compare
sergiocazzolato
approved these changes
May 6, 2026
|
Wed May 6 20:59:20 UTC 2026 Failures:Preparing:
Executing:
Restoring:
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 (without variants) of the below tests you wish to run (unskip plus test list must be valid yaml)
|
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.
Now that snapd and the
prompting-clientsupport prompting foraudio-record, addapparmor-prompting-integration-testsvariants to exercise it end-to-end.Lifespan "forever" is exercised thoroughly elsewhere, so use lifespan "timespan" to check rule persistence, especially since triggering prompts for
audio-recordis comparatively simple and quick to check.Also, since
audio-recordconstraints have no path/path-pattern, there's no way to namespace rules between tests, so we need to ensure they are cleaned between runs.These are inspired by the
audio-recordsmoke test, since there's no WirePlumber available in spread VMs to trigger a prompt the way it happens in practice (on resolute+).This relies on the
prompting-clientshipping this fix: canonical/prompting-client#308So in the meantime, we may need to switch this spread test to use the
latest/edgechannel of theprompting-client, and switch back tolatest/stablelater. We similarly need thelatest/edgechannel to land #16931, so it's probably worth making this switch in a dedicated PR, then landing these two PRs, then later reverting the one which switched channels.This work is tracked internally by https://warthogs.atlassian.net/browse/SNAPDENG-36765
unskip: