Skip to content

Conversation

@Hook25
Copy link
Collaborator

@Hook25 Hook25 commented Aug 7, 2025

Description

The point of inheriting them from only DISPLAY having processes was to target user processes. This prioritizes them (as we do know the username) but still proceeds till we run out of options. Additionally this also calculates XDG_RUNTIME_DIR and DBUS_SESSION_BUS_ADDRESS always if it wasn't found anywhere to (try) to make the job start anyway with a sane-ish value

Resolved issues

Fixes: CHECKBOX-1990
Fixes: #2058

Documentation

N/A

Tests

Launch a container and define the XDG_RUNTIME_DIR envvar. After this PR the function will read it correctly, before it won't.

LiaoU3 and others added 2 commits August 1, 2025 10:42
The point of inheriting them from only DISPLAY having processes was to
target user processes. This prioritizes them (as we do know the
username) but still proceeds till we run out of options. Additionally
this also calculates XDG_RUNTIME_DIR and DBUS_SESSION_BUS_ADDRESS always
if it wasn't found anywhere to (try) to make the job start anyway with a
  sane-ish value
@Hook25 Hook25 force-pushed the xdg_runtime_dir_missing branch from 614829b to 2afd9aa Compare August 7, 2025 13:48
@codecov
Copy link

codecov bot commented Aug 7, 2025

Codecov Report

❌ Patch coverage is 95.78059% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.90%. Comparing base (30f498f) to head (551d8e3).
⚠️ Report is 102 commits behind head on main.

Files with missing lines Patch % Lines
providers/base/bin/reboot_check_test.py 92.20% 2 Missing and 4 partials ⚠️
...ckbox-ng/plainbox/impl/session/remote_assistant.py 83.33% 1 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2067      +/-   ##
==========================================
+ Coverage   51.66%   51.90%   +0.24%     
==========================================
  Files         386      387       +1     
  Lines       41492    41679     +187     
  Branches     7711     7743      +32     
==========================================
+ Hits        21435    21635     +200     
+ Misses      19293    19275      -18     
- Partials      764      769       +5     
Flag Coverage Δ
checkbox-ng 70.90% <83.33%> (+0.12%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Collaborator

@fernando79513 fernando79513 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TYVM for this PR!
I think the strategy to inherit the envvars is much clearer now, and the detailed comments are appreciated.

Could you run the audio/alsa-playback in one of the DUTs that failed the test before to be sure that your change fixes the issue?

@LiaoU3
Copy link
Contributor

LiaoU3 commented Aug 8, 2025

@LiaoU3
Copy link
Contributor

LiaoU3 commented Aug 8, 2025

My sideload job

id: audio/alsa-playback
_summary: Playback works
_purpose:
 Check if sound is played through ALSA on the default output
_steps:
 1. Make sure speakers or headphones are connected to the device
 2. Commence the test
_verification:
 Did you hear the sound?
plugin: user-interact-verify
depends: audio/detect-playback-devices
environ: ALSA_CONFIG_PATH ALSADEVICE
flags: also-after-suspend
command:
 set -x
 echo "$XDG_RUNTIME_DIR"
 echo "$DBUS_SESSION_BUS_ADDRESS"
 alsa_test playback -d 5
category_id: com.canonical.plainbox::audio
estimated_duration: 10

It seems fixed on 202309-32029 with server 22.04

-----------------------------[ Running job 3 / 3 ]------------------------------
-------------------------------[ Playback works ]-------------------------------
ID: com.canonical.certification::audio/alsa-playback
Category: Audio tests
--------------------------------------------------------------------------------
Purpose:

Check if sound is played through ALSA on the default output

Steps:

1. Make sure speakers or headphones are connected to the device
2. Commence the test

Pick an action
    => press ENTER to continue
  c => add a comment
  s => skip this job
  q => save the session and quit
[csq]: 
+ echo /run/user/1000
+ echo unix:path=/run/user/1000/bus
+ alsa_test playback -d 5
/run/user/1000
unix:path=/run/user/1000/bus
ALSA lib conf.c:4016:(snd_config_hooks_call) function snd_config_hook_load_for_all_cards returned error: File exists
Failed to change volume: Master mixer not found.

It also seems fixed on 202412-36092 with server 24.04

-----------------------------[ Running job 3 / 3 ]------------------------------
-------------------------------[ Playback works ]-------------------------------
ID: com.canonical.certification::audio/alsa-playback
Category: Audio tests
--------------------------------------------------------------------------------
Purpose:

Check if sound is played through ALSA on the default output

Steps:

1. Make sure speakers or headphones are connected to the device
2. Commence the test

Pick an action
    => press ENTER to continue
  c => add a comment
  s => skip this job
  q => save the session and quit
[csq]: 
+ echo /run/user/1000
+ echo unix:path=/run/user/1000/bus
+ alsa_test playback -d 5
/run/user/1000
unix:path=/run/user/1000/bus
Verification:

Did you hear the sound?

Please decide what to do next:
  outcome: job needs verification
  comments: none
Pick an action
  c => add a comment
  p => set outcome to pass
  f => set outcome to fail
  s => set outcome to skip
  r => re-run this job
    => set suggested outcome [job passed]
[cpfsr]: 
--------------------------------------------------------------------------------
Outcome: job passed

Overall, the PR could fix the problem! Thank you @Hook25

@Hook25 Hook25 force-pushed the xdg_runtime_dir_missing branch from e04a1b4 to 551d8e3 Compare August 8, 2025 09:57
Copy link
Collaborator

@fernando79513 fernando79513 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM +1!

@fernando79513 fernando79513 merged commit a012133 into main Aug 8, 2025
33 checks passed
@fernando79513 fernando79513 deleted the xdg_runtime_dir_missing branch August 8, 2025 10:40
stanley31huang pushed a commit that referenced this pull request Aug 14, 2025
* Add $XDG_RUNTIME_DIR for alsa tests

* Inherit envvars more aggressively

The point of inheriting them from only DISPLAY having processes was to
target user processes. This prioritizes them (as we do know the
username) but still proceeds till we run out of options. Additionally
this also calculates XDG_RUNTIME_DIR and DBUS_SESSION_BUS_ADDRESS always
if it wasn't found anywhere to (try) to make the job start anyway with a
  sane-ish value

* Python 3.5 :(

* Also test the bailout mechanism

---------

Co-authored-by: liaou3 <[email protected]>
bladernr pushed a commit that referenced this pull request Aug 28, 2025
* Add $XDG_RUNTIME_DIR for alsa tests

* Inherit envvars more aggressively

The point of inheriting them from only DISPLAY having processes was to
target user processes. This prioritizes them (as we do know the
username) but still proceeds till we run out of options. Additionally
this also calculates XDG_RUNTIME_DIR and DBUS_SESSION_BUS_ADDRESS always
if it wasn't found anywhere to (try) to make the job start anyway with a
  sane-ish value

* Python 3.5 :(

* Also test the bailout mechanism

---------

Co-authored-by: liaou3 <[email protected]>
stanley31huang pushed a commit that referenced this pull request Oct 3, 2025
* Add $XDG_RUNTIME_DIR for alsa tests

* Inherit envvars more aggressively

The point of inheriting them from only DISPLAY having processes was to
target user processes. This prioritizes them (as we do know the
username) but still proceeds till we run out of options. Additionally
this also calculates XDG_RUNTIME_DIR and DBUS_SESSION_BUS_ADDRESS always
if it wasn't found anywhere to (try) to make the job start anyway with a
  sane-ish value

* Python 3.5 :(

* Also test the bailout mechanism

---------

Co-authored-by: liaou3 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

$XDG_RUNTIME_DIR is not set in checkbox control session

4 participants