tests: fix bug: lack of option sof-logger in tests

[arikgreen]

Add option -s (sof-logger) to tests for fix issue
sof-test/case-lib/lib.sh: line 955: [: -eq: unary operator expected

# Disable logging when available...
if [ ${OPT_VAL['s']} -eq 0 ]; then
    return 0
fi

before fix: run
after fix: run

[marc-hb]

Duplicating the same -s code in every test is not very "scalable", it wasn't a good design in the first place.

Have you considered using SOF_LOGGING == 'none' instead?

Also, maybe that code could be moved to case-lib/lib.sh.

if [ ${OPT_VAL['s']} -eq 0 ]; then [: -eq: unary operator expected

This is missing quotes BTW, it should be:

if [ "${OPT_VAL['s']}" -eq 0 ]; then

shellcheck should tell us that.

[lgirdwood]

@arikgreen any update for review comments ?

[arikgreen]

Duplicating the same -s code in every test is not very "scalable", it wasn't a good design in the first place.

Have you considered using SOF_LOGGING == 'none' instead?

Also, maybe that code could be moved to case-lib/lib.sh.

Yes, you right duplicate same code isn't good technic. Maybe it is a right time to fix it and move the code to the lib.sh.
I prepare the fix.

[arikgreen]

@arikgreen any update for review comments ?

sorry for late but I was sick for last week.

[arikgreen]

@marc-hb I decided implement a better quick fix

[marc-hb]

LGTM but I think there's a less subtle way.

Also, the PR description is now completely out of date with the commit message (that's because of the unusual "force-push" way SOF uses GitHub zephyrproject-rtos/zephyr#39194 - I digress)

[marc-hb]

That difference between [ ] and [[ ]] is too subtle IMHO, I think most people won't know it. You could just do this instead: Wrong suggestion, see below.

Suggested change

	if [[ ${OPT_VAL['s']} -eq 0 ]]; then
	if [ "${OPT_VAL['s']}" = 0 ]; then

[marc-hb]

This should be correct but please test it:

Suggested change

	if [[ ${OPT_VAL['s']} -eq 0 ]]; then
	if [ -n "${OPT_VAL['s']}" ] && [ "${OPT_VAL['s']}" -eq 0 ]; then

[marc-hb]

Actually, your [[ ]] solution and my = solution are both wrong.

• All tests that support the -s flag have OPT_VAL['s'] equals to 1 by default. Because the default is to collect logs when you don't pass -s (sorry for the double and triple negations, not all of them sof-test's fault)

• Tests that do NOT support -s flag and have OPT_VAL['s'] undefined must of course consistently default to collecting logs too.

So, undefined OPT_VAL['s'] must be equivalent to OPT_VAL['s'] = 1: both must collect logs by default. The current code emits an ugly -eq warning but it is functionally correct.

Both the [[ ]] solution and my = solution make undefined equivalent to 0, which stops collecting logs and hides errors in tests that do not support -s. It would unfortunately not be the first time:

• Stop ignoring various logger failures #373
• Check for a valid trace for ALL tests #297
• https://github.com/thesofproject/sof-test/issues?q=label%3A%22False%20Pass%20%2F%20green%20failure%22

How did you test this?