Re: [PATCH 1/6] tests/qemu-iotests: Improve the check for GNU sed

[Thomas Huth <thuth@redhat.com>]

On 08/02/2022 12.46, Hanna Reitz wrote:
> On 08.02.22 11:13, Thomas Huth wrote:
>> Instead of failing the iotests if GNU sed is not available (or skipping
>> them completely in the check-block.sh script), it would be better to
>> simply skip the bash-based tests, so that the python-based tests could
>> still be run. Thus add the check for BusyBox sed to common.rc and mark
>> the tests as "not run" if GNU sed is not available. Then we can also
>> remove the sed checks from the check-block.sh script.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   tests/check-block.sh         | 12 ------------
>>   tests/qemu-iotests/common.rc | 26 +++++++++++++-------------
>>   2 files changed, 13 insertions(+), 25 deletions(-)
>>
>> diff --git a/tests/check-block.sh b/tests/check-block.sh
>> index 720a46bc36..af0c574812 100755
>> --- a/tests/check-block.sh
>> +++ b/tests/check-block.sh
>> @@ -52,18 +52,6 @@ if LANG=C bash --version | grep -q 'GNU bash, version 
>> [123]' ; then
>>       skip "bash version too old ==> Not running the qemu-iotests."
>>   fi
>> -if ! (sed --version | grep 'GNU sed') > /dev/null 2>&1 ; then
> 
> This specifically tests for `sed`, whereas...

There was a check for "gsed" one line later:

  if ! command -v gsed >/dev/null 2>&1; then

... so the check-block.sh script ran the iotests also if "sed" was not GNU, 
but gsed was available.

> [...]
> 
>> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
>> index 9885030b43..9ea504810c 100644
>> --- a/tests/qemu-iotests/common.rc
>> +++ b/tests/qemu-iotests/common.rc
>> @@ -17,17 +17,27 @@
>>   # along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>   #
>> +# bail out, setting up .notrun file
>> +_notrun()
>> +{
>> +    echo "$*" >"$OUTPUT_DIR/$seq.notrun"
>> +    echo "$seq not run: $*"
>> +    status=0
>> +    exit
>> +}
>> +
>> +# We need GNU sed for the iotests. Make sure to not use BusyBox sed
>> +# which says that "This is not GNU sed version 4.0"
>>   SED=
>>   for sed in sed gsed; do
>> -    ($sed --version | grep 'GNU sed') > /dev/null 2>&1
>> +    ($sed --version | grep -v "not GNU sed" | grep 'GNU sed') > /dev/null 
>> 2>&1
> 
> ...this will accept `gsed`, too.  The problem is that many bash iotests just 
> use `sed` instead of `$SED`, so I think if we let this do the gatekeeping, 
> then we should change this to just check for `sed`.

I think we should be fine - at least for the tests in the "auto" group. 
Otherwise we would have seen test failures on non-Linux systems like *BSD 
earlier already.

  Thomas