Re: automated test? (was Re: Linux 2.6.17.7)

Re: automated test? (was Re: Linux 2.6.17.7)

[Chuck Ebbert]

In-Reply-To: <20060726142932.GE23701@stusta.de>

On Wed, 26 Jul 2006 16:29:32 +0200, Adrian Bunk wrote:

> The real problem is:
> How do we get some testing coverage of -stable kernels by users to catch 
> issues?
> And compile errors are the least of my worries.

The problem with the current method of releasing patch candidates is
that it's too hard to test them.  I would suggest:

        1.  In addition to posting all the patches separately to L-K,
            post a combined patch.  Have it change the makefile so it
            says 2.6.X.Y-rcZ; that way if an oops gets posted we know
            what the codebase was.  If the patch is too big, put it
            on a website.

        2.  Make the separate patches available on a website in Quilt
            format like Andrew does with -mm.  (Just like (1) above,
            make sure it changes the kernel version.)  This makes it
            easier for testers to fix individual patches.

        3.  Keep posting -rc's until nobody reports problems.

It's easy to generate (1) from (2):

        a.      Untar the quilt patchset into the new directory.
                Make sure the old and new dirs are subdirectories
                of some common directory, are identical and they
                are dist-clean.
        b.      mv broken-out patches
        c.      quilt push -a -q
        d.      cd ..
        e.      diff -uprN -X ignorefiles old new >old.new.patch
                -- ignorefiles contains two lines:
                        .pc
                        patches

-- 
Chuck

Re: automated test? (was Re: Linux 2.6.17.7)

[David Lang]

On Wed, 26 Jul 2006, Adrian Bunk wrote:

> On Wed, Jul 26, 2006 at 03:39:49PM +0100, Andrew de Quincey wrote:
>> On Wednesday 26 July 2006 15:29, Adrian Bunk wrote:
>> ...
>>> The real problem is:
>>> How do we get some testing coverage of -stable kernels by users to catch
>>> issues?
>>> And compile errors are the least of my worries.
>>
>> Yeah - I believe some people did test the DVB -stable patches, but obviously
>> without the budget-av driver compile option enabled, so it didn't compile
>> that code. DVB supports quite a few cards, so its easy to accidentally leave
>> off one of the options when doing a mass compile of all drivers.
>>
>> The only thing I can think of would be to require -stable patch submitters to
>> supply a list of CONFIG options that must be on to enable compilation of the
>> new code so people know exactly how to enable it for testing... but obviously
>> since those would be manually specified, they can be wrong too. But at least
>> it would show they'd thought about it a bit....
>
> This helps only with compilation errors, which are as I said the least
> of my worries.

however, this can be automaticly tested (or at least the config options 
specified can be tested)

> But does the hardware driven by this driver work?
> And if it does, is there a bug in the patch that causes the kernel to
> crash after some hours?

this cannot be tested by anyone who doesn't use the driver in question, and 
frequently that includes the programmer who makes the changes.

there's no way to protect against a logic error in a driver, however it is 
possible to put a safety net in place to catch compile problems due to mistakes.

should such a net be put into place? (along with asking submitters what compile 
options are relavent to their code), or is this a mistake that is rare enough to 
just live with it when it happens?

David Lang

Re: automated test? (was Re: Linux 2.6.17.7)

[Andrew de Quincey]

On Wednesday 26 July 2006 16:00, Adrian Bunk wrote:
> On Wed, Jul 26, 2006 at 03:39:49PM +0100, Andrew de Quincey wrote:
> > On Wednesday 26 July 2006 15:29, Adrian Bunk wrote:
> >...
> >
> > > The real problem is:
> > > How do we get some testing coverage of -stable kernels by users to
> > > catch issues?
> > > And compile errors are the least of my worries.
> >
> > Yeah - I believe some people did test the DVB -stable patches, but
> > obviously without the budget-av driver compile option enabled, so it
> > didn't compile that code. DVB supports quite a few cards, so its easy to
> > accidentally leave off one of the options when doing a mass compile of
> > all drivers.
> >
> > The only thing I can think of would be to require -stable patch
> > submitters to supply a list of CONFIG options that must be on to enable
> > compilation of the new code so people know exactly how to enable it for
> > testing... but obviously since those would be manually specified, they
> > can be wrong too. But at least it would show they'd thought about it a
> > bit....
>
> This helps only with compilation errors, which are as I said the least
> of my worries.
>
> But does the hardware driven by this driver work?
> And if it does, is there a bug in the patch that causes the kernel to
> crash after some hours?

That is a major problem with DVB development; no developer has all the 
hardware (in fact there is lots of hardware no developer has regular access 
to). Non-developers do test stuff in our HG repository, but again, the test 
coverage is very quite, since only some people do this, and most 
non-developers have only a few pieces of hardware. 

Many people wait till it comes out in the vanilla kernel... which is why there 
were so many bugfixes from me in 2.6.17.7, as it was the first real test of 
several new features/card supports.

Re: automated test? (was Re: Linux 2.6.17.7)

[Adrian Bunk]

On Wed, Jul 26, 2006 at 03:39:49PM +0100, Andrew de Quincey wrote:
> On Wednesday 26 July 2006 15:29, Adrian Bunk wrote:
>...
> > The real problem is:
> > How do we get some testing coverage of -stable kernels by users to catch
> > issues?
> > And compile errors are the least of my worries.
> 
> Yeah - I believe some people did test the DVB -stable patches, but obviously 
> without the budget-av driver compile option enabled, so it didn't compile 
> that code. DVB supports quite a few cards, so its easy to accidentally leave 
> off one of the options when doing a mass compile of all drivers.
> 
> The only thing I can think of would be to require -stable patch submitters to 
> supply a list of CONFIG options that must be on to enable compilation of the 
> new code so people know exactly how to enable it for testing... but obviously 
> since those would be manually specified, they can be wrong too. But at least 
> it would show they'd thought about it a bit....

This helps only with compilation errors, which are as I said the least 
of my worries.

But does the hardware driven by this driver work?
And if it does, is there a bug in the patch that causes the kernel to 
crash after some hours?

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: automated test? (was Re: Linux 2.6.17.7)

[Andrew de Quincey]

On Wednesday 26 July 2006 15:29, Adrian Bunk wrote:
> On Wed, Jul 26, 2006 at 03:10:02PM +0100, Andrew de Quincey wrote:
> > On Wednesday 26 July 2006 14:02, Adrian Bunk wrote:
> >...
> >
> > > What bothers me more is that noone tested this patch against the kernel
> > > it was applied against.
> > >
> > > The submitter didn't test it works (he didn't even test the
> > > compilation).
> >
> > Yes I did - I didn't test the final generated patch unfortunately since I
> > assumed it worked. The kernel I _meant_ to diff against worked perfectly
> > :(
>
> Sorry if this was wrong, it wasn't meant against you personally.
>
> Things do go wrong. That's life.
> And you aren't the first person who sent a patch that broke the
> compilation of the next -stable kernel.

thanks. I feel a little bit better :)

> The real problem is:
> How do we get some testing coverage of -stable kernels by users to catch
> issues?
> And compile errors are the least of my worries.

Yeah - I believe some people did test the DVB -stable patches, but obviously 
without the budget-av driver compile option enabled, so it didn't compile 
that code. DVB supports quite a few cards, so its easy to accidentally leave 
off one of the options when doing a mass compile of all drivers.

The only thing I can think of would be to require -stable patch submitters to 
supply a list of CONFIG options that must be on to enable compilation of the 
new code so people know exactly how to enable it for testing... but obviously 
since those would be manually specified, they can be wrong too. But at least 
it would show they'd thought about it a bit....

Re: automated test? (was Re: Linux 2.6.17.7)

[Adrian Bunk]

On Wed, Jul 26, 2006 at 03:10:02PM +0100, Andrew de Quincey wrote:
> On Wednesday 26 July 2006 14:02, Adrian Bunk wrote:
>...
> > What bothers me more is that noone tested this patch against the kernel
> > it was applied against.
> >
> > The submitter didn't test it works (he didn't even test the compilation).
> 
> Yes I did - I didn't test the final generated patch unfortunately since I 
> assumed it worked. The kernel I _meant_ to diff against worked perfectly :(

Sorry if this was wrong, it wasn't meant against you personally.

Things do go wrong. That's life.
And you aren't the first person who sent a patch that broke the 
compilation of the next -stable kernel.

The real problem is:
How do we get some testing coverage of -stable kernels by users to catch 
issues?
And compile errors are the least of my worries.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: automated test? (was Re: Linux 2.6.17.7)

[Andrew de Quincey]

On Wednesday 26 July 2006 14:02, Adrian Bunk wrote:
> On Tue, Jul 25, 2006 at 09:47:43AM -0700, David Lang wrote:
> > On Tue, 25 Jul 2006, Andrew de Quincey wrote:
> > >On Tuesday 25 July 2006 10:55, Arnaud Patard wrote:
> > >>Greg KH <gregkh@suse.de> writes:
> > >>
> > >>Hi,
> > >>
> > >>>We (the -stable team) are announcing the release of the 2.6.17.7
> > >>> kernel.
> > >>
> > >>Sorry, but doesn't compile if DVB_BUDGET_AV is set :(
> > >>
> > >>>Andrew de Quincey:
> > >>>      v4l/dvb: Fix budget-av frontend detection
> > >
> > >In fact it is just this patch causing the problem:
> >
> > <SNIP>
> >
> > >Sorry, I had so much work going on in that area I must have diffed the
> > >wrong
> > >kernel when I created this patch. :(
> >
> > is it reasonable to have an aotomated test figure out what config options
> > are relavent to a patch (or patchset) and test compile all the
> > combinations to catch this sort of mistake?
>
> If you think about it, you'll notice it's definitely not reasonable:
>
> #include <linux/module.h> brings you a dependency on 5 config options.
> #include <linux/pci.h> brings you a dependency on 6 config options.
>
> By only including these two headers you are at 2048 combinations.
> The number of valid configurations will be lower, but 500 test compiles
> sound realistically.
>
> With have a dozen #include's you might need more than a million test
> compiles.
>
> With a dozen #include's, you might need a trilion [1] test compiles.
>
>
> Compile errors are quickly catched and don't cause any serious problem.
>
> What bothers me more is that noone tested this patch against the kernel
> it was applied against.
>
> The submitter didn't test it works (he didn't even test the compilation).

Yes I did - I didn't test the final generated patch unfortunately since I 
assumed it worked. The kernel I _meant_ to diff against worked perfectly :(

Re: automated test? (was Re: Linux 2.6.17.7)

[Adrian Bunk]

On Tue, Jul 25, 2006 at 09:47:43AM -0700, David Lang wrote:
> On Tue, 25 Jul 2006, Andrew de Quincey wrote:
> 
> >On Tuesday 25 July 2006 10:55, Arnaud Patard wrote:
> >>Greg KH <gregkh@suse.de> writes:
> >>
> >>Hi,
> >>
> >>>We (the -stable team) are announcing the release of the 2.6.17.7 kernel.
> >>
> >>Sorry, but doesn't compile if DVB_BUDGET_AV is set :(
> >>
> >>>Andrew de Quincey:
> >>>      v4l/dvb: Fix budget-av frontend detection
> >
> >
> >In fact it is just this patch causing the problem:
> <SNIP>
> >Sorry, I had so much work going on in that area I must have diffed the 
> >wrong
> >kernel when I created this patch. :(
> 
> is it reasonable to have an aotomated test figure out what config options 
> are relavent to a patch (or patchset) and test compile all the combinations 
> to catch this sort of mistake?

If you think about it, you'll notice it's definitely not reasonable:

#include <linux/module.h> brings you a dependency on 5 config options.
#include <linux/pci.h> brings you a dependency on 6 config options.

By only including these two headers you are at 2048 combinations.
The number of valid configurations will be lower, but 500 test compiles 
sound realistically.

With have a dozen #include's you might need more than a million test 
compiles.

With a dozen #include's, you might need a trilion [1] test compiles.


Compile errors are quickly catched and don't cause any serious problem.

What bothers me more is that noone tested this patch against the kernel 
it was applied against.

The submitter didn't test it works (he didn't even test the compilation).

No user tested it.

Currently, -stable kernels get an 48 hours review on linux-kernel but 
zero testing.

How could this be improved?
Longer review/testing time?
Offer them as also one 2.6.17.7-rc1 patch?

> David Lang

cu
Adrian

[1] American English

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: automated test? (was Re: Linux 2.6.17.7)

[David Lang]

On Tue, 25 Jul 2006, Matthias Andree wrote:

> On Tue, 25 Jul 2006, Arjan van de Ven wrote:
>
>> well you can do such a thing withing statistical bounds; however... if
>> the patch already is in -git (as is -stable policy normally).. it should
>> have been found there already...
>
> The sad facts I learned from Debian bug #212762 (not kernel related) that
> culminated in CVE-2005-2335 (remote root exploit against older
> fetchmail) and from various qmail bugs Guninski discovered:
>
> - a bug need not necessarily be found soon after introduction
>
> - a bug report may not convey the hint "look at this NOW, the shit
>  already hit the fan"
>  (sorry, I meant to write: look NOW, it's urgent and important)
>
> - an automated test to catch non-trivial mistakes is non-trivial in
>  itself, and - what I've seen with another project I was involved with,
>  and more often than I found amusing - is that the test itself can be
>  buggy causing bogus results.
>
> That doesn't mean I object to automated tests, but "it should have been
> found by now" (because the source is open, someone could have tested it,
> whatever) just doesn't work.

what I was intending with my origional question was a series of simple 'does it 
compile' tests that try all the config options that are affected by the patchset 
in question. the purpose being to catch simple errors like the one here where 
the patch was diffed against the wrong tree and the result doesn't compile

i.e. if the change is the the e1000 driver, try compiling a kernel with it on, 
off, and as a module

obviously such a test can't be done on the huge -rc patches (they would approach 
an exhaustive test of all config permutations), but for -stable patches (or 
better still the indivdual patches that form a -stable release)

so the first part of the question boils down to

1. Given a patch that modifies file X is it possible to know what config options 
could be affected?

and the second part would be

2. would it make sense for the LTP or one of the big compile farms to do a 
series of compiles prior to the release of a -stable kernel to catch mistakes 
like this?

David Lang

Re: automated test? (was Re: Linux 2.6.17.7)

[Matthias Andree]

On Tue, 25 Jul 2006, Arjan van de Ven wrote:

> well you can do such a thing withing statistical bounds; however... if
> the patch already is in -git (as is -stable policy normally).. it should
> have been found there already...

The sad facts I learned from Debian bug #212762 (not kernel related) that
culminated in CVE-2005-2335 (remote root exploit against older
fetchmail) and from various qmail bugs Guninski discovered:

- a bug need not necessarily be found soon after introduction

- a bug report may not convey the hint "look at this NOW, the shit
  already hit the fan"
  (sorry, I meant to write: look NOW, it's urgent and important)

- an automated test to catch non-trivial mistakes is non-trivial in
  itself, and - what I've seen with another project I was involved with,
  and more often than I found amusing - is that the test itself can be
  buggy causing bogus results.

That doesn't mean I object to automated tests, but "it should have been
found by now" (because the source is open, someone could have tested it,
whatever) just doesn't work.

-- 
Matthias Andree

Re: automated test? (was Re: Linux 2.6.17.7)

[Arnaud Patard]

David Lang <dlang@digitalinsight.com> writes:

> On Tue, 25 Jul 2006, Andrew de Quincey wrote:
>
>> On Tuesday 25 July 2006 10:55, Arnaud Patard wrote:
>>> Greg KH <gregkh@suse.de> writes:
>>>
>>> Hi,
>>>
>>>> We (the -stable team) are announcing the release of the 2.6.17.7 kernel.
>>>
>>> Sorry, but doesn't compile if DVB_BUDGET_AV is set :(
>>>
>>>> Andrew de Quincey:
>>>>       v4l/dvb: Fix budget-av frontend detection
>>
>>
>> In fact it is just this patch causing the problem:
> <SNIP>
>> Sorry, I had so much work going on in that area I must have diffed the wrong
>> kernel when I created this patch. :(
>
> is it reasonable to have an aotomated test figure out what config options are 
> relavent to a patch (or patchset) and test compile all the combinations to catch 
> this sort of mistake?

you'll probably need to find some heuristics which may be quite hard to
do. It would be easier imho to use some scripts like this one :
http://developer.osdl.org/~cherry/compile/ 

It's often enough to catch compile failures


Arnaud Patard

>
> David Lang

Re: automated test? (was Re: Linux 2.6.17.7)

[Andrew de Quincey]

On Tuesday 25 July 2006 19:42, Jean Delvare wrote:
> Hi Michael,
>
> > The fix can be found here... We'll need this queued up for 2.6.17.8 ...
> >  I have already attached this inline in a prior email, not sure how many
> > people have seen that yet...
> >
> > You can also get it from here:
> >
> > http://linuxtv.org/~mkrufky/stable/2.6.17.y/budget-av-compile-fix.patch
>
> Yes, it fixed the compilation cleanly for me (on both i386 and x86_64).
> I can't test beyond that though, as I don't have that specific hardware.

Great thanks for testing!

As for not having specific hardware for testing, I have the same problem. Its 
a continuing problem with DVB and the source of many regressions :(

Re: automated test? (was Re: Linux 2.6.17.7)

[Jean Delvare]

Hi Michael,

> The fix can be found here... We'll need this queued up for 2.6.17.8 ...
>  I have already attached this inline in a prior email, not sure how many
> people have seen that yet...
> 
> You can also get it from here:
> 
> http://linuxtv.org/~mkrufky/stable/2.6.17.y/budget-av-compile-fix.patch

Yes, it fixed the compilation cleanly for me (on both i386 and x86_64).
I can't test beyond that though, as I don't have that specific hardware.

Thanks,
-- 
Jean Delvare

Re: automated test? (was Re: Linux 2.6.17.7)

[Michael Krufky]

Arjan van de Ven wrote:
> On Tue, 2006-07-25 at 09:47 -0700, David Lang wrote:
>> On Tue, 25 Jul 2006, Andrew de Quincey wrote:
>>
>>> On Tuesday 25 July 2006 10:55, Arnaud Patard wrote:
>>>> Greg KH <gregkh@suse.de> writes:
>>>>
>>>> Hi,
>>>>
>>>>> We (the -stable team) are announcing the release of the 2.6.17.7 kernel.
>>>> Sorry, but doesn't compile if DVB_BUDGET_AV is set :(
>>>>
>>>>> Andrew de Quincey:
>>>>>       v4l/dvb: Fix budget-av frontend detection
>>>
>>> In fact it is just this patch causing the problem:
>> <SNIP>
>>> Sorry, I had so much work going on in that area I must have diffed the wrong
>>> kernel when I created this patch. :(
>> is it reasonable to have an aotomated test figure out what config options are 
>> relavent to a patch (or patchset) and test compile all the combinations to catch 
>> this sort of mistake?
> 
> well you can do such a thing withing statistical bounds; however... if
> the patch already is in -git (as is -stable policy normally).. it should
> have been found there already...
> 
> 
In this case it isn't quite that simple...

The DVB tree is in the midst of tuner refactoring, and somehow the diff
was generated against the wrong tree.

The fix can be found here... We'll need this queued up for 2.6.17.8 ...
 I have already attached this inline in a prior email, not sure how many
people have seen that yet...

You can also get it from here:

http://linuxtv.org/~mkrufky/stable/2.6.17.y/budget-av-compile-fix.patch

---

[PATCH 2.6.17.7] Fix budget-av compile failure

From: Andrew de Quincey <adq_dvb@lidskialf.net>

Currently I am doing lots of refactoring work in the dvb tree. This
bugfix became necessary to fix 2.6.17 whilst I was in the middle of this
work. Unfortunately after I tested the original code for the patch, I
generated the diff against the wrong tree (I accidentally used a tree
with part of the refactoring code in it). This resulted in the reported
compile errors because that tree (a) was incomplete, and (b) used
features which are simply not in the mainline kernel yet.

Many apologies for the error and problems this has caused. :(

Signed-off-by: Andrew de Quincey <adq_dvb@lidskialf.net>
Signed-off-by: Michael Krufky <mkrufky@linuxtv.org>

diff -Naur linux-2.6.17.7.orig/drivers/media/dvb/ttpci/budget-av.c
linux-2.6.17.7/drivers/media/dvb/ttpci/budget-av.c
--- linux-2.6.17.7.orig/drivers/media/dvb/ttpci/budget-av.c 2006-07-25
14:53:19.000000000 +0100
+++ linux-2.6.17.7/drivers/media/dvb/ttpci/budget-av.c 2006-07-25
15:25:32.000000000 +0100
@@ -58,6 +58,7 @@
 	struct tasklet_struct ciintf_irq_tasklet;
 	int slot_status;
 	struct dvb_ca_en50221 ca;
+	u8 reinitialise_demod:1;
 };

 /* GPIO Connections:
@@ -214,8 +215,9 @@
 	while (--timeout > 0 && ciintf_read_attribute_mem(ca, slot, 0) != 0x1d)
 		msleep(100);

-	/* reinitialise the frontend */
-	dvb_frontend_reinitialise(budget_av->budget.dvb_frontend);
+	/* reinitialise the frontend if necessary */
+	if (budget_av->reinitialise_demod)
+		dvb_frontend_reinitialise(budget_av->budget.dvb_frontend);

 	if (timeout <= 0)
 	{
@@ -1064,12 +1066,10 @@
 		fe = tda10021_attach(&philips_cu1216_config,
 				     &budget_av->budget.i2c_adap,
 				     read_pwm(budget_av));
-		if (fe) {
-			fe->ops.tuner_ops.set_params = philips_cu1216_tuner_set_params;
-		}
 		break;

 	case SUBID_DVBC_KNC1_PLUS:
+		budget_av->reinitialise_demod = 1;
 		fe = tda10021_attach(&philips_cu1216_config,
 				     &budget_av->budget.i2c_adap,
 				     read_pwm(budget_av));



-- 
Michael Krufky

Re: automated test? (was Re: Linux 2.6.17.7)

[Arjan van de Ven]

On Tue, 2006-07-25 at 09:47 -0700, David Lang wrote:
> On Tue, 25 Jul 2006, Andrew de Quincey wrote:
> 
> > On Tuesday 25 July 2006 10:55, Arnaud Patard wrote:
> >> Greg KH <gregkh@suse.de> writes:
> >>
> >> Hi,
> >>
> >>> We (the -stable team) are announcing the release of the 2.6.17.7 kernel.
> >>
> >> Sorry, but doesn't compile if DVB_BUDGET_AV is set :(
> >>
> >>> Andrew de Quincey:
> >>>       v4l/dvb: Fix budget-av frontend detection
> >
> >
> > In fact it is just this patch causing the problem:
> <SNIP>
> > Sorry, I had so much work going on in that area I must have diffed the wrong
> > kernel when I created this patch. :(
> 
> is it reasonable to have an aotomated test figure out what config options are 
> relavent to a patch (or patchset) and test compile all the combinations to catch 
> this sort of mistake?

well you can do such a thing withing statistical bounds; however... if
the patch already is in -git (as is -stable policy normally).. it should
have been found there already...


-- 
if you want to mail me at work (you don't), use arjan (at) linux.intel.com

automated test? (was Re: Linux 2.6.17.7)

[David Lang]

On Tue, 25 Jul 2006, Andrew de Quincey wrote:

> On Tuesday 25 July 2006 10:55, Arnaud Patard wrote:
>> Greg KH <gregkh@suse.de> writes:
>>
>> Hi,
>>
>>> We (the -stable team) are announcing the release of the 2.6.17.7 kernel.
>>
>> Sorry, but doesn't compile if DVB_BUDGET_AV is set :(
>>
>>> Andrew de Quincey:
>>>       v4l/dvb: Fix budget-av frontend detection
>
>
> In fact it is just this patch causing the problem:
<SNIP>
> Sorry, I had so much work going on in that area I must have diffed the wrong
> kernel when I created this patch. :(

is it reasonable to have an aotomated test figure out what config options are 
relavent to a patch (or patchset) and test compile all the combinations to catch 
this sort of mistake?

David Lang