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

[James Almer]

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.