[RFC] fate: Add --skip-tests configure option for omitting FATE tests

Message ID 1475793356-2549-1-git-send-email-diego@biurrun.de
State Superseded
Headers show

Commit Message

Diego Biurrun Oct. 6, 2016, 10:35 p.m.
This can be useful in known-broken scenarios like miscompilation
by legacy compilers and similar.
---

Yeah, one could come up with more elaborate solutions, but this already
works and we have cases where it could be useful right away...

 configure      | 3 +++
 tests/Makefile | 2 ++
 2 files changed, 5 insertions(+)

Comments

Martin Storsjö Oct. 7, 2016, 7:45 a.m. | #1
On Fri, 7 Oct 2016, Diego Biurrun wrote:

> This can be useful in known-broken scenarios like miscompilation
> by legacy compilers and similar.
> ---
>
> Yeah, one could come up with more elaborate solutions, but this already
> works and we have cases where it could be useful right away...

This looks like a good improvement over the current situation, thanks!

I guess this should be included in the output sent to fate as well (and 
shown there in some way)? Initially I guess one can add it to the comments 
column.

// Martin
Diego Biurrun Oct. 7, 2016, 9 a.m. | #2
On Fri, Oct 07, 2016 at 10:45:47AM +0300, Martin Storsjö wrote:
> On Fri, 7 Oct 2016, Diego Biurrun wrote:
> 
> > This can be useful in known-broken scenarios like miscompilation
> > by legacy compilers and similar.
> > ---
> >
> > Yeah, one could come up with more elaborate solutions, but this already
> > works and we have cases where it could be useful right away...
> 
> This looks like a good improvement over the current situation, thanks!
> 
> I guess this should be included in the output sent to fate as well (and 
> shown there in some way)? Initially I guess one can add it to the comments 
> column.

It shows up in the configure line; do you want it shown somewhere else?

And yeah, I'd probably add it to the comments field of my own FATE
instances if I used the option.

Diego
Martin Storsjö Oct. 7, 2016, 9:06 a.m. | #3
On Fri, 7 Oct 2016, Diego Biurrun wrote:

> On Fri, Oct 07, 2016 at 10:45:47AM +0300, Martin Storsjö wrote:
>> On Fri, 7 Oct 2016, Diego Biurrun wrote:
>> 
>> > This can be useful in known-broken scenarios like miscompilation
>> > by legacy compilers and similar.
>> > ---
>> >
>> > Yeah, one could come up with more elaborate solutions, but this already
>> > works and we have cases where it could be useful right away...
>> 
>> This looks like a good improvement over the current situation, thanks!
>> 
>> I guess this should be included in the output sent to fate as well (and 
>> shown there in some way)? Initially I guess one can add it to the comments 
>> column.
>
> It shows up in the configure line; do you want it shown somewhere else?
>
> And yeah, I'd probably add it to the comments field of my own FATE
> instances if I used the option.

I thought of a separate column next to the comments (which is already used 
for other things in many setups), but plugging it into the comments 
probably is good for the first step.

// Martin
Martin Storsjö Oct. 7, 2016, 6:14 p.m. | #4
On Fri, 7 Oct 2016, Martin Storsjö wrote:

> On Fri, 7 Oct 2016, Diego Biurrun wrote:
>
>> On Fri, Oct 07, 2016 at 10:45:47AM +0300, Martin Storsjö wrote:
>>> On Fri, 7 Oct 2016, Diego Biurrun wrote:
>>> 
>>> > This can be useful in known-broken scenarios like miscompilation
>>> > by legacy compilers and similar.
>>> > ---
>>> >
>>> > Yeah, one could come up with more elaborate solutions, but this already
>>> > works and we have cases where it could be useful right away...
>>> 
>>> This looks like a good improvement over the current situation, thanks!
>>> 
>>> I guess this should be included in the output sent to fate as well (and 
>>> shown there in some way)? Initially I guess one can add it to the comments 
>>> column.
>>
>> It shows up in the configure line; do you want it shown somewhere else?
>>
>> And yeah, I'd probably add it to the comments field of my own FATE
>> instances if I used the option.
>
> I thought of a separate column next to the comments (which is already used 
> for other things in many setups), but plugging it into the comments 
> probably is good for the first step.

In any case, if I was unclear, this patch LGTM.

// Martin
Martin Storsjö Oct. 13, 2016, 8:34 p.m. | #5
On Fri, 7 Oct 2016, Martin Storsjö wrote:

> On Fri, 7 Oct 2016, Martin Storsjö wrote:
>
>> On Fri, 7 Oct 2016, Diego Biurrun wrote:
>>
>>> On Fri, Oct 07, 2016 at 10:45:47AM +0300, Martin Storsjö wrote:
>>>> On Fri, 7 Oct 2016, Diego Biurrun wrote:
>>>> 
>>>> > This can be useful in known-broken scenarios like miscompilation
>>>> > by legacy compilers and similar.
>>>> > ---
>>>> >
>>>> > Yeah, one could come up with more elaborate solutions, but this already
>>>> > works and we have cases where it could be useful right away...
>>>> 
>>>> This looks like a good improvement over the current situation, thanks!
>>>> 
>>>> I guess this should be included in the output sent to fate as well (and 
>>>> shown there in some way)? Initially I guess one can add it to the 
> comments 
>>>> column.
>>>
>>> It shows up in the configure line; do you want it shown somewhere else?
>>>
>>> And yeah, I'd probably add it to the comments field of my own FATE
>>> instances if I used the option.
>>
>> I thought of a separate column next to the comments (which is already used 
>> for other things in many setups), but plugging it into the comments 
>> probably is good for the first step.
>
> In any case, if I was unclear, this patch LGTM.

Any further comments on this, or interest in pushing it? I'd like to take 
it into use.

// Martin
Diego Biurrun Oct. 13, 2016, 10:50 p.m. | #6
On Thu, Oct 13, 2016 at 11:34:05PM +0300, Martin Storsjö wrote:
> On Fri, 7 Oct 2016, Martin Storsjö wrote:
> >On Fri, 7 Oct 2016, Martin Storsjö wrote:
> >>On Fri, 7 Oct 2016, Diego Biurrun wrote:
> >>>On Fri, Oct 07, 2016 at 10:45:47AM +0300, Martin Storsjö wrote:
> >>>>On Fri, 7 Oct 2016, Diego Biurrun wrote:
> >>>>
> >>>>> This can be useful in known-broken scenarios like miscompilation
> >>>>> by legacy compilers and similar.
> >>>>> ---
> >>>>>
> >>>>> Yeah, one could come up with more elaborate solutions, but this already
> >>>>> works and we have cases where it could be useful right away...
> >>>>
> >>>>This looks like a good improvement over the current situation, thanks!
> >>>>
> >>>>I guess this should be included in the output sent to fate as well
> >>>>(and shown there in some way)? Initially I guess one can add it to
> >>>>the
> >comments
> >>>>column.
> >>>
> >>>It shows up in the configure line; do you want it shown somewhere else?
> >>>
> >>>And yeah, I'd probably add it to the comments field of my own FATE
> >>>instances if I used the option.
> >>
> >>I thought of a separate column next to the comments (which is already
> >>used for other things in many setups), but plugging it into the comments
> >>probably is good for the first step.
> >
> >In any case, if I was unclear, this patch LGTM.
> 
> Any further comments on this, or interest in pushing it? I'd like to take it
> into use.

I was kinda waiting for Janne to make a comment since I spoke with him
about this idea in Berlin. But I guess this is good to go.

I had one idea how to improve/change this: I could automatically add the
"fate-" prefix to the test name to shorten the command line. So

--skip-tests="fate-foo-test fate-bar-test"

vs

--skip-tests="foo-test bar-test"

What would you prefer?

Diego
Martin Storsjö Oct. 14, 2016, 6:31 a.m. | #7
On Fri, 14 Oct 2016, Diego Biurrun wrote:

> On Thu, Oct 13, 2016 at 11:34:05PM +0300, Martin Storsjö wrote:
>> On Fri, 7 Oct 2016, Martin Storsjö wrote:
>> >On Fri, 7 Oct 2016, Martin Storsjö wrote:
>> >>On Fri, 7 Oct 2016, Diego Biurrun wrote:
>> >>>On Fri, Oct 07, 2016 at 10:45:47AM +0300, Martin Storsjö wrote:
>> >>>>On Fri, 7 Oct 2016, Diego Biurrun wrote:
>> >>>>
>> >>>>> This can be useful in known-broken scenarios like miscompilation
>> >>>>> by legacy compilers and similar.
>> >>>>> ---
>> >>>>>
>> >>>>> Yeah, one could come up with more elaborate solutions, but this already
>> >>>>> works and we have cases where it could be useful right away...
>> >>>>
>> >>>>This looks like a good improvement over the current situation, thanks!
>> >>>>
>> >>>>I guess this should be included in the output sent to fate as well
>> >>>>(and shown there in some way)? Initially I guess one can add it to
>> >>>>the
>> >comments
>> >>>>column.
>> >>>
>> >>>It shows up in the configure line; do you want it shown somewhere else?
>> >>>
>> >>>And yeah, I'd probably add it to the comments field of my own FATE
>> >>>instances if I used the option.
>> >>
>> >>I thought of a separate column next to the comments (which is already
>> >>used for other things in many setups), but plugging it into the comments
>> >>probably is good for the first step.
>> >
>> >In any case, if I was unclear, this patch LGTM.
>> 
>> Any further comments on this, or interest in pushing it? I'd like to take it
>> into use.
>
> I was kinda waiting for Janne to make a comment since I spoke with him
> about this idea in Berlin. But I guess this is good to go.

Ah, I see.

> I had one idea how to improve/change this: I could automatically add the
> "fate-" prefix to the test name to shorten the command line. So
>
> --skip-tests="fate-foo-test fate-bar-test"
>
> vs
>
> --skip-tests="foo-test bar-test"
>
> What would you prefer?

That'd probably be useful.

// Martin

Patch

diff --git a/configure b/configure
index 7d4a4ab..be27c1a 100755
--- a/configure
+++ b/configure
@@ -347,6 +347,7 @@  Developer options (useful when working on Libav itself):
   --random-seed=VALUE      seed value for --enable/disable-random
   --disable-valgrind-backtrace do not print a backtrace under Valgrind
                            (only applies to --disable-optimizations builds)
+  --skip-tests=TESTS       whitespace-separated list of FATE tests to skip
 
 NOTE: Object files are built at the place where configure is launched.
 EOF
@@ -1808,6 +1809,7 @@  CMDLINE_SET="
     pkg_config_flags
     random_seed
     samples
+    skip_tests
     sysinclude
     sysroot
     target_exec
@@ -5313,6 +5315,7 @@  SLIB_INSTALL_EXTRA_LIB=${SLIB_INSTALL_EXTRA_LIB}
 SLIB_INSTALL_EXTRA_SHLIB=${SLIB_INSTALL_EXTRA_SHLIB}
 VERSION_SCRIPT_POSTPROCESS_CMD=${VERSION_SCRIPT_POSTPROCESS_CMD}
 SAMPLES:=${samples:-\$(LIBAV_SAMPLES)}
+SKIP_TESTS=$skip_tests
 EOF
 
 get_version(){
diff --git a/tests/Makefile b/tests/Makefile
index 36a3a72..4bcdc4a 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -142,6 +142,8 @@  endif
 
 FATE_UTILS = base64 tiny_psnr
 
+FATE := $(filter-out $(SKIP_TESTS),$(FATE))
+
 fate: $(FATE)
 
 $(FATE): $(FATE_UTILS:%=tests/%$(HOSTEXESUF))