Skip to content

fix: tests: SRC tests catch aplay warnings and refactor#1351

Open
ekurdybx wants to merge 1 commit intothesofproject:mainfrom
ekurdybx:fix-src-tests
Open

fix: tests: SRC tests catch aplay warnings and refactor#1351
ekurdybx wants to merge 1 commit intothesofproject:mainfrom
ekurdybx:fix-src-tests

Conversation

@ekurdybx
Copy link
Contributor

@ekurdybx ekurdybx commented Mar 9, 2026

What's changed:

  • check for aplay warnings (ex. about wrong sample rate)
  • check aplay and arecord exit codes (these tests do not exit on errors automatically, because we want to go through all the tests in the loop, even if previous ones have failed)
  • refactor the check_soundfiles_for_glitches function
  • add logs for cleaner and more readable results
  • move generated audio files used for testing from logs dir to avoid "littering" in the test artifacts
  • add default parameters

@ekurdybx ekurdybx force-pushed the fix-src-tests branch 15 times, most recently from 683253f to 4fe141b Compare March 10, 2026 11:51
Catch aplay incorrect sample rate warnings and refactor tests

Signed-off-by: Emilia Kurdybelska <emiliax.kurdybelska@intel.com>
@ekurdybx ekurdybx marked this pull request as ready for review March 10, 2026 12:18
@ekurdybx ekurdybx requested review from a team, golowanow, lgirdwood and marc-hb as code owners March 10, 2026 12:18
check_for_aplay_warnings()
{
dlogi "$1"
if echo "$1" | grep -q "Warning:"; then
Copy link
Collaborator

Choose a reason for hiding this comment

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


# Check aplay output for warnings
# Arguments: 1-aplay output
check_for_aplay_warnings()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
check_for_aplay_warnings()
check_for_warnings()

# shellcheck disable=SC2086
arecord $SOF_ALSA_OPTS $SOF_ARECORD_OPTS $1 & PID=$!
# shellcheck disable=SC2181
if [ $? -ne 0 ]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

$? will never be useful when you background a command. To get the status of a backgrounded command you need to wait $PID

# shellcheck disable=SC2086
aplay $SOF_ALSA_OPTS $SOF_APLAY_OPTS $2
aplay_output=$(aplay $SOF_ALSA_OPTS $SOF_APLAY_OPTS $2 2>&1)
# shellcheck disable=SC2181
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not just:

aplay_output=$(aplay $SOF_ALSA_OPTS $SOF_APLAY_OPTS $2 2>&1) || errors=$((errors+1))

fi
# shellcheck disable=SC2086
aplay $SOF_ALSA_OPTS $SOF_APLAY_OPTS $2
aplay_output=$(aplay $SOF_ALSA_OPTS $SOF_APLAY_OPTS $2 2>&1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Capturing output loses "real-time" feedback. You can (and should) have both with something like this:

set -o pipefail
aplay $SOF_ALSA_OPTS $SOF_APLAY_OPTS $2 2>&1 | tee _.log || errors=$((errors+1))

check_warnings _.log

There are other ways:

https://stackoverflow.com/questions/1221833/pipe-output-and-capture-exit-status-in-bash

@marc-hb
Copy link
Collaborator

marc-hb commented Mar 10, 2026

Forgot to mention this:

The "real" fix should be in aplay itself, not in sof-test. As in: aplay --fatal-errors --fatal-warnings. @kv2019i is it something that ALSA could implement in the (distant...) future?

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.

2 participants