[FFmpeg-devel] [PATCHv2] fate: Add an option for disabling the 2k/4k tests

[Martin Storsjö]

On Fri, 13 Dec 2019, James Almer wrote:

> On 12/13/2019 7:22 PM, Martin Storsjö wrote:
>> On Fri, 13 Dec 2019, Carl Eugen Hoyos wrote:
>> 
>>>
>>>
>>>> Am 13.12.2019 um 09:34 schrieb Martin Storsjö <martin at martin.st>:
>>>>
>>>>> On Thu, 12 Dec 2019, James Almer wrote:
>>>>>
>>>>>> On 12/12/2019 7:03 PM, Martin Storsjö wrote:
>>>>>>> On Wed, 11 Dec 2019, Martin Storsjö wrote:
>>>>>>>> On Wed, 11 Dec 2019, Carl Eugen Hoyos wrote:
>>>>>>>>
>>>>>>>> Am Mi., 11. Dez. 2019 um 09:39 Uhr schrieb Martin Storsjö 
>>>>>>> <martin at martin.st>:
>>>>>>>>>
>>>>>>>>> When testing on a memory limited system, these tests consume a
>>>>>>>>> significant amount of memory and can often fail if testing by
>>>>>>>>> running
>>>>>>>>> multiple processes in parallel.
>>>>>>>>> ---
>>>>>>>>> Adjusted to use ALLYES instead of a -yes-yes construct.
>>>>>>>>>
>>>>>>>>> Also moved the 2k tests to the same option.
>>>>>>>>> ---
>>>>>>>>>  configure             | 3 +++
>>>>>>>>>  tests/fate/seek.mak   | 3 ++-
>>>>>>>>>  tests/fate/vcodec.mak | 5 +++--
>>>>>>>>>  3 files changed, 8 insertions(+), 3 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/configure b/configure
>>>>>>>>> index ca7137f341..922cd8d0ee 100755
>>>>>>>>> --- a/configure
>>>>>>>>> +++ b/configure
>>>>>>>>> @@ -482,6 +482,7 @@ Developer options (useful when working on
>>>>>>>>> FFmpeg 
>>>>>>> itself):
>>>>>>>>>    --ignore-tests=TESTS     comma-separated list (without "fate-"
>>>>>>>>> prefix
>>>>>>>>>                             in the name) of tests whose result is
>>>>>>>>> ignored
>>>>>>>>>    --enable-linux-perf      enable Linux Performance Monitor API
>>>>>>>>> +  --disable-large-tests    disable tests that use a large
>>>>>>>>> amount of 
>>>>>>> memory
>>>>>>>>
>>>>>>>> I would have suggested to control this when running the tests, if
>>>>>>>> the 
>>>>>>> configure
>>>>>>>> setting makes sense, it should at least be possible to change the
>>>>>>>> setting 
>>>>>>> when
>>>>>>>> calling make.
>>>>>>>> Or is that possible anyway?
>>>>>>>
>>>>>>> It's possible to do e.g. "make fate CONFIG_LARGE_TESTS=no"; any
>>>>>>> var=value assignment on the make command line overrides any
>>>>>>> var=othervalue assignment within the makefiles themselves, but that
>>>>>>> doesn't seem very convenient.
>>>>>>>
>>>>>>> But I'd like to have it as a configure option, to easily be able to
>>>>>>> set it e.g. in a fate setup.
>>>>>> Any further opinions on this one - is it ok to go ahead with it in
>>>>>> this
>>>>>> form, or are changes requested?
>>>>>
>>>>> Configure option is fine if it can also be overridden from the command
>>>>> line at runtime (like --samples and SAMPLES).
>>>>
>>>> Well, it can be overridden at runtime, but it's not with a very
>>>> convenient name (the CONFIG_* variable). Is that ok?
>>>
>>> Ideally, this would be possible with:
>>> make BIG=no fate
>> 
>> That requires a bit more extra intermediate variables.
>> 
>> One way of doing it would be this:
>> 
>> # Default, overriden by any "make BIG=no fate"
>> BIG=$(CONFIG_LARGE_TESTS)
>> ...
>> TESTS-$(ENCDEC components)-$(BIG) += some-tests
>> TESTS += TESTS-yes-yes
>> 
>> While it earlier was requested to use $(ALLYES ...) instead of the
>> -yes-yes construct.
>> 
>> Or to keep using ALLYES, we'd need yet another intermediate variable:
>> 
>> # Default, overriden by any "make BIG=no fate"
>> BIG=$(CONFIG_LARGE_TESTS)
>> # The same as BIG above, but with a CONFIG_ prefix
>> CONFIG_BIG=$(BIG)
>> ...
>> TESTS-$(ALLYES components BIG) += some-tests
>> TESTS += TESTS-yes
>> 
>> 
>> 
>> James, what's your opinion on these two alternatives, if it should be
>> configurable with a different variable name?
>
> BIG is too generic and could be used for anything. LARGE_TESTS would be
> better, and would get rid of the need to add a new custom CONFIG_
> variable for the second example using ALLYES.

I intentionally meant to use a different variable for that, to 
differentiate between the configure-generated CONFIG_LARGE_TESTS from 
config.mak and the one that is set to pick up a potential user-supplied 
value from e.g. LARGE_TESTS, otherwise falling back on the configure 
generated value - I'm not sure if that really works if the first and last 
variable are the same, or if that ends up as an infinite recursion 
otherwise (CONFIG_LARGE_TESTS expands to LARGE_TESTS which expands to 
CONFIG_LARGE_TESTS).

// Martin