sclorg / container-common-scripts

Apache License 2.0
20 stars 45 forks source link

RFE: run-command function or similar #148

Open hhorak opened 4 years ago

hhorak commented 4 years ago

As suggested in https://github.com/sclorg/httpd-container/pull/94, there could be a generic ct_run_cmd function similar to what https://github.com/sclorg/httpd-container/ and some other images use -- such a function might check the output and exit code, might also print some standardized logging etc...

A note: we might get some more inspiration in beakerlib where rlRun has a similar function.

zmiklank commented 1 year ago

I think that this is now covered by ct_run_tests_from_testset. Could we close this?

hhorak commented 1 year ago

I meant something else here. Some function that we could use to run some important commands instead of running them and then checking the output. Such function could also log something useful. E.g. httpd-container uses:

function run() {
    cmd="$1"
    expected_res="${2:-0}"
    msg="${3:-Running command '$cmd'}"
    run_command "$cmd" "$expected_res" "$msg"
    res=$?
    test "$res" -eq "$expected_res" && res=0 || res=1
    ct_check_testcase_result $res
    return $res
}

Feel free to check also what rlRun from beakerlib does (the implementation is pretty complex, attached to many internals, so nothing we could steal, but might be taken as an inspiration): https://github.com/beakerlib/beakerlib/blob/a5f5610ed254e07edbe60ec695a0203e64e4fb24/src/testing.sh#L664-L756

I can also imagine that the arguments of this function would be logged to a specific file/variable and when the test case fails, we could return what commands were run, so it could help figuring out a reproducer. IOW, we would not get all the preparation steps, only the important commands.

I'm not saying we cannot live with this new function (we're fine without it till today), it could just make the test scripts a bit more clear and output likely more readable.

zmiklank commented 1 year ago

Thanks for explanation Honzo, now it makes sense to me and I agree. Let's leave this opened then.