Bug#847926: Bug#852820: Testsuite-Restrictions field is hard to use

[Ian Jackson]

Iain Lane writes ("Re: Bug#852820: Testsuite-Restrictions field is hard to 
use"):
> I don't, or not really - see below. I plan on using this field
> externally to choose between a couple of available backends to dispatch
> to when constructing autopkgtest invocations

Yes, I understand that.  But I think this field is too tailored to
your specific use case and too ripe for misuse.

> > https://anonscm.debian.org/cgit/autopkgtest/autopkgtest.git/plain/doc/README.package-tests.rst
> >  says:
> > Tests which declare unknown restrictions will be skipped. See below
> > for the defined restrictions.
> 
> So it would seem to me (unless I'm missing something in there like an
> exception for x-prefixed items that I can't see ATM) that this would be
> legal and as a result I was not really expecting arbitrary values to be
> present. Again, I'm not going to be doing that, so it's not a problem
> now and this is apparently not currently implemented in autopkgtest
> itself either, since dgit's tests are being run.

Only a few of dgit's tests have these restrictions.  The tests with
those restrictions are not run by Debian's CI.  They can be run in
ad-hoc situations of various kinds.

Declaring test restrictions, using a private bit of the namespace, in
this way, seems like an appropriate way to achieve that effect.

For the avoidance of doubt: I'm not saying that your program would
mishandle correct packages.  (Is it happy with dgit's test suite,
despite the presence of some un-runnable tests?)

But as I say this field seems to invite people to use it for deciding
whether the package is testable, but it is not useable for that
purpose.

To put it another way: why not publish the intersection of the
restrictions ?  Why not also publish the union or intersection of the
features ?  As I wrote:

> > > If we do need more information in Sources.gz then maybe the set of
> > > combinations of restrictions ought to be listed.  But then the
> > > features might be useful too.

ISTM that your real problem is that you don't have an efficient way to
access debian/tests/control.  Perhaps the right answer is to provide
an additional side channel for this information, rather than burdening
the Sources file.  Many many, non-testing-related, programs need to
read Sources.  I think the test metadata is rather too rich to express
compactly.

> > But I certainly agree that this should have probably been discussed
> > more widely, which is something I overlooked, sorry about that. And
> > agree completely with your two points above. So I'll be adding an
> > entry to the FAQ detailing the process to add new information to
> > the .dsc file (and probably to the .changes and other interchange
> > formats).
> 
> Yes, I concede that I should have posted on the list. I discussed with
> Martin first, but that is not enough. Sorry about that.

Thanks.

Ian.

Bug#847926: Bug#852820: Testsuite-Restrictions field is hard to use

[Iain Lane]

On Sat, Jan 28, 2017 at 05:47:44AM +0100, Guillem Jover wrote:
> Hi!
> 
> On Fri, 2017-01-27 at 15:58:28 +, Ian Jackson wrote:
> > Package: dpkg-dev
> > Version: 1.18.19
> > 
> > >From the patch from Iain Lane:
> > 
> >  +This field declares the comma-separated union of all test restrictions
> >  +(\fBRestrictions\fP fields in \fIdebian/tests/control\fP file).
> > 
> > This is quite awkward to use correctly, and is a hazard to correct
> > implementation.  For example, dgit's debian/tests/control contains
> > several tests marked with Dgit-specific Restrictions with
> > x-dgit-... names.  The result is that with 1.18.10ubuntu1, we would
> > see this in dgit's .dsc:
> > 
> >  Testsuite-Restrictions: x-dgit-intree-only x-dgit-git-only 
> > x-dgit-schroot-build
> > 
> > If not interpreted very carefully, this would give a test suite runner
> > the erroneous impression that none of the tests can be run.
> 
> Right, I see what you mean.

I don't, or not really - see below. I plan on using this field
externally to choose between a couple of available backends to dispatch
to when constructing autopkgtest invocations - ssh (nova) or something
less heavyweight like lxd. In this case unknown or uninteresting values
will simply be ignored. I suppose you're worried about somebody
intersecting the field with the supported restrictions of particular
backends or something like that. That's not what I plan to do, but the
README says:

> https://anonscm.debian.org/cgit/autopkgtest/autopkgtest.git/plain/doc/README.package-tests.rst
>  says:
> Tests which declare unknown restrictions will be skipped. See below
> for the defined restrictions.

So it would seem to me (unless I'm missing something in there like an
exception for x-prefixed items that I can't see ATM) that this would be
legal and as a result I was not really expecting arbitrary values to be
present. Again, I'm not going to be doing that, so it's not a problem
now and this is apparently not currently implemented in autopkgtest
itself either, since dgit's tests are being run.

> > Also, Iain's stated use case in #847926 does not require anything new
> > in the .dsc and hence Sources.gz.  All that is required to get the
> > full information (debian/tests/control) is to extract the source
> > package.  That extraction of the source package has to be done anyway
> > no matter how the tests will be run.
> 
> OTOH, the same could be said for several of the fields in the .dsc, such
> as Build-Depends, but we still list them because it's more convenient to
> not have to extract the source beforehand.

Exactly that. dpkg is constructing the information from the source
package, so it's always possible for somebody else to do the same work.
It's just much more convenient for it to be in Sources already.

> 
> > If we do need more information in Sources.gz then maybe the set of
> > combinations of restrictions ought to be listed.  But then the
> > features might be useful too.
> > 
> > I'm sorry that I'm reporting this bug now, due to happening to see the
> > message on debian-dpkg.  But really I think:
> > 
> >  * Changes to the .dsc format and hence to the Sources format should
> >be discussed more widely than a bug on debian-dpkg.
> > 
> >  * Changes to the handling of autopkgtest (DEP-8) metadata should be
> >discussed on autopkgtest-devel (or other DEP-8 related places).
> 
> TBH, I hesitated a bit before adding this, because this percolates
> into many other places. But considered that, while the information
> could be retrieved in other ways, it made life easier for test
> runners. And we can always remove it if it ends up being
> unnecessary/undesirable.

It's definitely going to be necessary and desirable for me.

> But I certainly agree that this should have probably been discussed
> more widely, which is something I overlooked, sorry about that. And
> agree completely with your two points above. So I'll be adding an
> entry to the FAQ detailing the process to add new information to
> the .dsc file (and probably to the .changes and other interchange
> formats).

Yes, I concede that I should have posted on the list. I discussed with
Martin first, but that is not enough. Sorry about that.

Cheers,

-- 
Iain Lane  [ i...@orangesquash.org.uk ]
Debian Developer   [ la...@debian.org ]
Ubuntu Developer   [ la...@ubuntu.com ]


signature.asc
Description: PGP signature

Bug#847926: Bug#852820: Testsuite-Restrictions field is hard to use

[Ian Jackson]

Guillem Jover writes ("Re: Bug#852820: Testsuite-Restrictions field is hard to 
use"):
> On Fri, 2017-01-27 at 15:58:28 +, Ian Jackson wrote:
> > If not interpreted very carefully, this would give a test suite runner
> > the erroneous impression that none of the tests can be run.
> 
> Right, I see what you mean.

Can you please add something to the documentation, sayinng that this
field MUST NOT be used to determine whether a package has any tests
that can be run ?  (Because it cannot be correctly so used.)

> > Also, Iain's stated use case in #847926 does not require anything new
> > in the .dsc and hence Sources.gz.  All that is required to get the
> > full information (debian/tests/control) is to extract the source
> > package.  That extraction of the source package has to be done anyway
> > no matter how the tests will be run.
> 
> OTOH, the same could be said for several of the fields in the .dsc, such
> as Build-Depends, but we still list them because it's more convenient to
> not have to extract the source beforehand.

Build-Depends is not a (perverse) atttempt at a summary of some other
pieces of metadata.

> TBH, I hesitated a bit before adding this, because this percolates
> into many other places. But considered that, while the information
> could be retrieved in other ways, it made life easier for test
> runners. And we can always remove it if it ends up being
> unnecessary/undesirable.

You have permanently assigned the name `Testsuite-Restrictions' for
this unnatural meaning, and you and Iain intend for Iain's software to
start relying on it (so withdrawing it would be troublesomw).
Changing published metadata formats is not so easy, I'm afraid.

> But I certainly agree that this should have probably been discussed
> more widely, which is something I overlooked, sorry about that. And
> agree completely with your two points above. So I'll be adding an
> entry to the FAQ detailing the process to add new information to
> the .dsc file (and probably to the .changes and other interchange
> formats).

Thanks,
Ian.

Bug#847926: Bug#852820: Testsuite-Restrictions field is hard to use

[Guillem Jover]

On Sat, 2017-01-28 at 05:47:44 +0100, Guillem Jover wrote:
> On Fri, 2017-01-27 at 15:58:28 +, Ian Jackson wrote:
> > I'm sorry that I'm reporting this bug now, due to happening to see the
> > message on debian-dpkg.  But really I think:
> > 
> >  * Changes to the .dsc format and hence to the Sources format should
> >be discussed more widely than a bug on debian-dpkg.
> > 
> >  * Changes to the handling of autopkgtest (DEP-8) metadata should be
> >discussed on autopkgtest-devel (or other DEP-8 related places).
> 
> But I certainly agree that this should have probably been discussed
> more widely, which is something I overlooked, sorry about that. And
> agree completely with your two points above. So I'll be adding an
> entry to the FAQ detailing the process to add new information to
> the .dsc file (and probably to the .changes and other interchange
> formats).

I've now updated the FAQ to include processes for field inclusions to
.dsc and .changes files:

  

Thanks,
Guillem

Bug#847926: Bug#852820: Testsuite-Restrictions field is hard to use

[Guillem Jover]

Hi!

On Fri, 2017-01-27 at 15:58:28 +, Ian Jackson wrote:
> Package: dpkg-dev
> Version: 1.18.19
> 
> >From the patch from Iain Lane:
> 
>  +This field declares the comma-separated union of all test restrictions
>  +(\fBRestrictions\fP fields in \fIdebian/tests/control\fP file).
> 
> This is quite awkward to use correctly, and is a hazard to correct
> implementation.  For example, dgit's debian/tests/control contains
> several tests marked with Dgit-specific Restrictions with
> x-dgit-... names.  The result is that with 1.18.10ubuntu1, we would
> see this in dgit's .dsc:
> 
>  Testsuite-Restrictions: x-dgit-intree-only x-dgit-git-only 
> x-dgit-schroot-build
> 
> If not interpreted very carefully, this would give a test suite runner
> the erroneous impression that none of the tests can be run.

Right, I see what you mean.

> Also, Iain's stated use case in #847926 does not require anything new
> in the .dsc and hence Sources.gz.  All that is required to get the
> full information (debian/tests/control) is to extract the source
> package.  That extraction of the source package has to be done anyway
> no matter how the tests will be run.

OTOH, the same could be said for several of the fields in the .dsc, such
as Build-Depends, but we still list them because it's more convenient to
not have to extract the source beforehand.

> If we do need more information in Sources.gz then maybe the set of
> combinations of restrictions ought to be listed.  But then the
> features might be useful too.
> 
> I'm sorry that I'm reporting this bug now, due to happening to see the
> message on debian-dpkg.  But really I think:
> 
>  * Changes to the .dsc format and hence to the Sources format should
>be discussed more widely than a bug on debian-dpkg.
> 
>  * Changes to the handling of autopkgtest (DEP-8) metadata should be
>discussed on autopkgtest-devel (or other DEP-8 related places).

TBH, I hesitated a bit before adding this, because this percolates
into many other places. But considered that, while the information
could be retrieved in other ways, it made life easier for test
runners. And we can always remove it if it ends up being
unnecessary/undesirable.

But I certainly agree that this should have probably been discussed
more widely, which is something I overlooked, sorry about that. And
agree completely with your two points above. So I'll be adding an
entry to the FAQ detailing the process to add new information to
the .dsc file (and probably to the .changes and other interchange
formats).

Thanks,
Guillem