Hi Pratik,

Thanks for adding a new test case.  Some comments:

> On 14 Nov 2025, at 01:41, Pratik Farkase via lists.openembedded.org 
> <[email protected]> wrote:
> +for test in check_check_export check_check; do
> +    if [ -x ./$test ]; then
> +        ./$test > /dev/null 2>&1
> +        if [ $? -eq 0 ]; then
> +            echo "PASS: $test"
> +        else
> +            echo "FAIL: $test"
> +        fi
> +    else
> +        echo "SKIP: $test"
> +    fi
> +done
> +
> +for test in test_output.sh test_check_nofork.sh 
> test_check_nofork_teardown.sh \
> +            test_xml_output.sh test_log_output.sh test_set_max_msg_size.sh \
> +            test_tap_output.sh; do
> +    if [ -x ./$test ]; then
> +        ./$test > /dev/null 2>&1
> +        if [ $? -eq 0 ]; then
> +            echo "PASS: $test"
> +        else
> +            echo "FAIL: $test"
> +        fi
> +    else
> +        echo "SKIP: $test"
> +    fi
> +done

This looks like the same loop, twice. Why not merge them?

> +    for binary in ${B}/tests/*; do
> +        if [ -f "$binary" ] && [ -x "$binary" ]; then
> +            case "$binary" in
> +                *.sh) ;;
> +                *) install -m 0755 "$binary" ${D}${PTEST_PATH}/tests/ || 
> true ;;

Don’t ||true, if it fails we want this to fail.

Also considering the lists of tests is hardcoded in run-ptest _and_ upstream 
hasn’t been changed in four years, it might be easier to just hardcode the list 
of binaries that we install instead of playing games trying to identify the 
right output.

> +            esac
> +        fi
> +    done
> +
> +    for script in test_output.sh test_check_nofork.sh 
> test_check_nofork_teardown.sh \
> +                  test_xml_output.sh test_log_output.sh 
> test_set_max_msg_size.sh \
> +                  test_tap_output.sh; do
> +        install -m 0755 ${S}/tests/$script ${D}${PTEST_PATH}/tests/
> +    done

This is just test/*.sh, right?

> +    install -m 0755 ${S}/tests/test_output_strings ${D}${PTEST_PATH}/tests/
> +
> +    if [ -f ${B}/tests/test_vars ]; then
> +        install -m 0644 ${B}/tests/test_vars ${D}${PTEST_PATH}/tests/
> +        sed -i \
> +            -e 's|if \[ x"[^"]*" != x"\." \];|if \[ 
> x"/usr/src/debug/libcheck/0.15.2/tests" != x"." \];|g' \
> +            -e 
> 's|SRCDIR="[^"]*"|SRCDIR="/usr/src/debug/libcheck/0.15.2/tests/"|g' \
> +            ${D}${PTEST_PATH}/tests/test_vars

Hard-coded paths that will break on upgrade.  ${TARGET_DBGSRC_DIR} is the 
variable you want.

> +RDEPENDS:${PN}-ptest += "bash libcheck"

You can remove this: I don’t see any scripts that use bash, and the dependency 
on libcheck will be generated automatically.

Thanks,
Ross
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#226522): 
https://lists.openembedded.org/g/openembedded-core/message/226522
Mute This Topic: https://lists.openembedded.org/mt/116286168/21656
Group Owner: [email protected]
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to