Message ID | 20231211221406.2385689-1-martin@martin.st |
---|---|
State | Superseded |
Headers | show |
Series | [FFmpeg-devel] fate: Allow running multiple rounds of tests with differing settings | expand |
Context | Check | Description |
---|---|---|
andriy/configure_x86 | warning | Failed to apply patch |
yinshiyou/configure_loongarch64 | warning | Failed to apply patch |
Le tiistaina 12. joulukuuta 2023, 0.14.06 EET Martin Storsjö a écrit : > This can be used to run tests multple times, with e.g. differing > QEMU settings, by adding something like this to the FATE configuration > file: > > target_exec="qemu-aarch64-static" > fate_targets="fate-checkasm fate-cpu" > > fate_environments="sve128 sve256 sve512" > sve128_env="QEMU_CPU=max,sve128=on" > sve256_env="QEMU_CPU=max,sve256=on" > sve512_env="QEMU_CPU=max,sve512=on" I'm fine with that, but for the sake of generality, shouldn't rather the entire target_exec prefix be indirected? Some runners may want to use command line flags rather than environment variables.
On Wed, 13 Dec 2023, Rémi Denis-Courmont wrote: > Le tiistaina 12. joulukuuta 2023, 0.14.06 EET Martin Storsjö a écrit : >> This can be used to run tests multple times, with e.g. differing >> QEMU settings, by adding something like this to the FATE configuration >> file: >> >> target_exec="qemu-aarch64-static" >> fate_targets="fate-checkasm fate-cpu" >> >> fate_environments="sve128 sve256 sve512" >> sve128_env="QEMU_CPU=max,sve128=on" >> sve256_env="QEMU_CPU=max,sve256=on" >> sve512_env="QEMU_CPU=max,sve512=on" > > I'm fine with that, but for the sake of generality, shouldn't rather the > entire target_exec prefix be indirected? Some runners may want to use > command line flags rather than environment variables. Yes - that's also doable. One can e.g. do this: --- target_exec="qemu-aarch64-static -cpu \$(MY_CPU)" fate_targets="fate-checkasm fate-cpu" fate_environments="sve128 sve256 sve512" sve128_env="MY_CPU=max,sve128=on" sve256_env="MY_CPU=max,sve256=on" sve512_env="MY_CPU=max,sve512=on" --- That way, one can also make the whole target_exec be e.g. \$(EXEC_CMD) and set the full command via the individual envs. It's not quite as comfortable, but should be doable and allow fully generic setups. (One could also consider just doing feat1_env="TARGET_EXEC=..." and not setting target_exec in the fate config at all - however that's probably not a good option and it has one surprising gotcha. The makefile level TARGET_EXEC variable needs to have a trailing space when it is set to a nonempty value, and injecting that might be annoying or ugly.) // Martin
On Wed, 13 Dec 2023, Martin Storsjö wrote: > On Wed, 13 Dec 2023, Rémi Denis-Courmont wrote: > >> Le tiistaina 12. joulukuuta 2023, 0.14.06 EET Martin Storsjö a écrit : >>> This can be used to run tests multple times, with e.g. differing >>> QEMU settings, by adding something like this to the FATE configuration >>> file: >>> >>> target_exec="qemu-aarch64-static" >>> fate_targets="fate-checkasm fate-cpu" >>> >>> fate_environments="sve128 sve256 sve512" >>> sve128_env="QEMU_CPU=max,sve128=on" >>> sve256_env="QEMU_CPU=max,sve256=on" >>> sve512_env="QEMU_CPU=max,sve512=on" >> >> I'm fine with that, but for the sake of generality, shouldn't rather the >> entire target_exec prefix be indirected? Some runners may want to use >> command line flags rather than environment variables. > > Yes - that's also doable. One can e.g. do this: > > --- > target_exec="qemu-aarch64-static -cpu \$(MY_CPU)" > fate_targets="fate-checkasm fate-cpu" > > fate_environments="sve128 sve256 sve512" > sve128_env="MY_CPU=max,sve128=on" > sve256_env="MY_CPU=max,sve256=on" > sve512_env="MY_CPU=max,sve512=on" > --- > > That way, one can also make the whole target_exec be e.g. \$(EXEC_CMD) and > set the full command via the individual envs. It's not quite as comfortable, > but should be doable and allow fully generic setups. If this sounds reasonable enough, I would go ahead and land this, together with https://patchwork.ffmpeg.org/project/ffmpeg/patch/20231127123118.3622784-1-martin@martin.st/. I guess this feature should have some documentation as well - I can draft up a v2 with docs before going ahead with it, if the feature overall looks acceptable. // Martin
diff --git a/tests/Makefile b/tests/Makefile index 444c09b3de..744dbcdfb3 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -305,8 +305,8 @@ $(FATE): export PROGSUF = $(PROGSSUF) $(FATE): export EXECSUF = $(EXESUF) $(FATE): export HOSTEXECSUF = $(HOSTEXESUF) $(FATE): $(FATE_UTILS:%=tests/%$(HOSTEXESUF)) | $(FATE_OUTDIRS) - @echo "TEST $(@:fate-%=%)" - $(Q)$(SRC_PATH)/tests/fate-run.sh $@ "$(TARGET_SAMPLES)" "$(TARGET_EXEC)" "$(TARGET_PATH)" '$(CMD)' '$(CMP)' '$(REF)' '$(FUZZ)' '$(THREADS)' '$(THREAD_TYPE)' '$(CPUFLAGS)' '$(CMP_SHIFT)' '$(CMP_TARGET)' '$(SIZE_TOLERANCE)' '$(CMP_UNIT)' '$(GEN)' '$(HWACCEL)' '$(REPORT)' '$(KEEP_FILES)' + @echo "TEST $(@:fate-%=%)$(FATE_SUFFIX)" + $(Q)$(SRC_PATH)/tests/fate-run.sh $@$(FATE_SUFFIX) "$(TARGET_SAMPLES)" "$(TARGET_EXEC)" "$(TARGET_PATH)" '$(CMD)' '$(CMP)' '$(REF)' '$(FUZZ)' '$(THREADS)' '$(THREAD_TYPE)' '$(CPUFLAGS)' '$(CMP_SHIFT)' '$(CMP_TARGET)' '$(SIZE_TOLERANCE)' '$(CMP_UNIT)' '$(GEN)' '$(HWACCEL)' '$(REPORT)' '$(KEEP_FILES)' fate-list: @printf '%s\n' $(sort $(FATE)) diff --git a/tests/fate.sh b/tests/fate.sh index 95408ea109..c04eb41cbe 100755 --- a/tests/fate.sh +++ b/tests/fate.sh @@ -101,7 +101,19 @@ compile_extra()( fate()( test "$build_only" = "yes" && return cd ${build} || return - ${make} ${makeopts_fate-${makeopts}} -k ${fate_targets} + if [ -n "${fate_environments}" ]; then + ret=0 + for e in ${fate_environments}; do + eval "curenv=\${${e}_env}" + echo Testing environment ${e}: ${curenv} + ${make} ${makeopts_fate-${makeopts}} -k ${fate_targets} FATE_SUFFIX=_${e} ${curenv} + cur_ret=$? + test $cur_ret != 0 && ret=$cur_ret + done + return $ret + else + ${make} ${makeopts_fate-${makeopts}} -k ${fate_targets} + fi ) clean(){