| Message ID | 1475793356-2549-1-git-send-email-diego@biurrun.de |
|---|---|
| State | Superseded |
| Headers | show |
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
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
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
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
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
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
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
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))