Bug#851232: debci: does not understand space-separated tests

[Simon McVittie]

Control: retitle -1 autopkgtest: confusing output when a multi-test stanza is 
skipped
Control: severity -1 minor
Control: tags -1 = patch
Control: forwarded -1 
https://salsa.debian.org/ci-team/autopkgtest/merge_requests/57

On Tue, 20 Mar 2018 at 20:41:08 +, Simon McVittie wrote:
> Perhaps this
> is just misleading output, using after-resolve as shorthand for
> "after-resolve and everything with the same dependencies"?

Yes, that was the problem. If autopkgtest skips a stanza with multiple
tests, it only logs the name of the first test.

For example:

Tests: one two
Restrictions: needs-root

Tests: foo bar
Restrictions: needs-more-cowbell

gets logged as:

one  SKIP Test needs root on testbed which is not available
foo  SKIP unknown restriction needs-more-cowbell

I interpreted this as debci or autopkgtest mis-parsing the tests control
file, so I think this is sufficiently confusing that it's worth fixing
autopkgtest so it logs similar (and somewhat redundant) messages about
the tests "two" and "bar". See attached (or the Salsa MR).

smcv
>From 4b076cb714269f05ac00affc916bd4c15db87313 Mon Sep 17 00:00:00 2001
From: Simon McVittie 
Date: Fri, 26 Jul 2019 10:26:45 +0100
Subject: [PATCH] testdesc: Report each unsupported test individually

Previously, if a stanza in d/tests/control had unknown fields or
unsupported restrictions, we would log that for the first test in the
stanza (only), for example:

Tests: one two
Restrictions: needs-root

Tests: foo bar
Restrictions: needs-more-cowbell

would be logged as

one  SKIP Test needs root on testbed which is not available
foo  SKIP unknown restriction needs-more-cowbell

This is sufficiently confusing that it led me to file an invalid bug
report about autopkgtest not supporting space-separated tests. If we
report the Unsupported exception for each test-case, it becomes clearer
what's really going on:

one  SKIP Test needs root on testbed which is not available
two  SKIP Test needs root on testbed which is not available
foo  SKIP unknown restriction needs-more-cowbell
bar  SKIP unknown restriction needs-more-cowbell

Closes: #851232
Signed-off-by: Simon McVittie 
---
 lib/testdesc.py   | 15 ++-
 tests/autopkgtest | 20 +++-
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/lib/testdesc.py b/lib/testdesc.py
index d68edaf..84d24db 100644
--- a/lib/testdesc.py
+++ b/lib/testdesc.py
@@ -512,12 +512,17 @@ def parse_debian_source(srcdir, testbed_caps, testbed_arch, control_path=None,
 '*', 'test-name feature incompatible with Tests')
 test_dir = record.get('Tests-directory', 'debian/tests')
 
-_debian_check_unknown_fields(test_names[0], record)
 for n in test_names:
-test = Test(n, os.path.join(test_dir, n), None,
-restrictions, features, depends, [], [], synth_depends)
-test.check_testbed_compat(testbed_caps)
-tests.append(test)
+try:
+_debian_check_unknown_fields(n, record)
+test = Test(n, os.path.join(test_dir, n), None,
+restrictions, features, depends, [], [], synth_depends)
+test.check_testbed_compat(testbed_caps)
+except Unsupported as u:
+u.report()
+some_skipped = True
+else:
+tests.append(test)
 elif 'Test-command' in record:
 command = record['Test-command']
 (depends, synth_depends) = _parse_debian_depends(
diff --git a/tests/autopkgtest b/tests/autopkgtest
index 2d4d025..f5eb1ed 100755
--- a/tests/autopkgtest
+++ b/tests/autopkgtest
@@ -247,9 +247,10 @@ class DebTestsAll:
 def test_isolation(self):
 '''isolation restrictions'''
 
-p = self.build_src('Tests: ic\nDepends:\nRestrictions: isolation-container\n\n'
+p = self.build_src('Tests: ic ic2\nDepends:\nRestrictions: isolation-container\n\n'
'Tests: im\nDepends:\nRestrictions: isolation-machine\n',
{'ic': '#!/bin/sh\necho container ok',
+'ic2': '#!/bin/sh\necho container2 ok',
 'im': '#!/bin/sh\necho machine ok'})
 
 (code, out, err) = self.runtest(['-B', p])
@@ -263,10 +264,14 @@ class DebTestsAll:
 
 if not self.has_isolation_container:
 self.assertRegex(out, r'ic\s+SKIP .*container', out)
+self.assertRegex(out, r'ic2\s+SKIP .*container', out)
 self.assertNotIn('container ok', out)
+self.assertNotIn('container2 ok', out)
 

Bug#851232: debci: does not understand space-separated tests

[Paul Gevers]

Dear Simon,

On 20-03-18 22:08, Paul Gevers wrote:
> Hi Simon,
> 
> On 20-03-18 21:41, Simon McVittie wrote:
>> ci.debian.net can't currently run these tests anyway (they're marked as
>> needing isolation-machine) so they'd be skipped regardless. Perhaps this
>> is just misleading output, using after-resolve as shorthand for
>> "after-resolve and everything with the same dependencies"?
> 
> I bet this is it, but I'll check

I wonder if my fix for 832751 won't also fix this issue. Shall we check
after it becomes available (maybe today already)?

Paul

Bug#851232: debci: does not understand space-separated tests

[Paul Gevers]

Hi Simon,

On 20-03-18 21:41, Simon McVittie wrote:
> ci.debian.net can't currently run these tests anyway (they're marked as
> needing isolation-machine) so they'd be skipped regardless. Perhaps this
> is just misleading output, using after-resolve as shorthand for
> "after-resolve and everything with the same dependencies"?

I bet this is it, but I'll check

> https://autopkgtest.ubuntu.com/packages/nss-mdns/bionic/amd64 (which I
> think is also debci)

I believe Ubuntu uses it's own framework, but with a similar UI.

Paul



signature.asc
Description: OpenPGP digital signature

Bug#851232: debci: does not understand space-separated tests

[Simon McVittie]

On Tue, 20 Mar 2018 at 16:22:15 +0100, Paul Gevers wrote:
> On Fri, 13 Jan 2017 07:12:36 + Simon McVittie  wrote:
> > The autopkgtest spec[1] says "Test names are separated by comma and/or
> > whitespace" (which I admit is weird, but it's what it says) and the
> > reference implementation implements that:
> > 
> > 
> > debian/tests/control in nss-mdns 0.10-8 has space-separated Tests (because
> > I misremembered what is allowed) and it looks as though ci.debian.net
> > responds to this by only running the first test per stanza:
> > https://ci.debian.net/data/packages/unstable/amd64/n/nss-mdns/20170113_063407.autopkgtest.log.gz
> > 
> > There is no practical impact right now for this particular package,
> > because I don't think nss-mdns' tests can be relied on to work in a
> > container anyway, so they're all skipped.
> 
> Do you believe the issue is still present? It seems you removed the test
> from nss-mdns and when I test this (albeit within the autopkgtest test
> suite) it seems currently to work as expected.

https://ci.debian.net/data/autopkgtest/unstable/amd64/n/nss-mdns/91/log.gz says:

autopkgtest [01:40:14]: testing package nss-mdns version 0.10-8
autopkgtest [01:40:14]: build not needed
after-resolveSKIP Test requires machine-level isolation but testbed 
does not provide that
nss-behaviourSKIP Test requires machine-level isolation but testbed 
does not provide that
autopkgtest [01:40:14]:  summary
after-resolveSKIP Test requires machine-level isolation but testbed 
does not provide that
nss-behaviourSKIP Test requires machine-level isolation but testbed 
does not provide that

but there are more tests than just those two:

Tests: after-resolve before-resolve jessie-after-resolve jessie-before-resolve 
multiarch-purge multiarch-remove remove-reinstall
Depends: avahi-daemon, avahi-utils, dbus
Restrictions: allow-stderr, breaks-testbed, isolation-machine, needs-root

Tests: nss-behaviour
Depends: dbus
Restrictions: allow-stderr, breaks-testbed, isolation-machine, needs-root

ci.debian.net can't currently run these tests anyway (they're marked as
needing isolation-machine) so they'd be skipped regardless. Perhaps this
is just misleading output, using after-resolve as shorthand for
"after-resolve and everything with the same dependencies"?

https://autopkgtest.ubuntu.com/packages/nss-mdns/bionic/amd64 (which I
think is also debci) supports isolation-machine, and that does run all
the tests, except for multiarch-* which are patched out (they don't
work on Ubuntu's infrastructure).

smcv

Bug#851232: debci: does not understand space-separated tests

[Paul Gevers]

Control: tags -1 moreinfo

Hi Simon,

On Fri, 13 Jan 2017 07:12:36 + Simon McVittie  wrote:
> The autopkgtest spec[1] says "Test names are separated by comma and/or
> whitespace" (which I admit is weird, but it's what it says) and the
> reference implementation implements that:
> 
> 
> debian/tests/control in nss-mdns 0.10-8 has space-separated Tests (because
> I misremembered what is allowed) and it looks as though ci.debian.net
> responds to this by only running the first test per stanza:
> https://ci.debian.net/data/packages/unstable/amd64/n/nss-mdns/20170113_063407.autopkgtest.log.gz
> 
> There is no practical impact right now for this particular package,
> because I don't think nss-mdns' tests can be relied on to work in a
> container anyway, so they're all skipped.

Do you believe the issue is still present? It seems you removed the test
from nss-mdns and when I test this (albeit within the autopkgtest test
suite) it seems currently to work as expected.

Paul



signature.asc
Description: OpenPGP digital signature

Bug#851232: debci: does not understand space-separated tests

[Antonio Terceiro]

On Fri, Jan 13, 2017 at 07:12:36AM +, Simon McVittie wrote:
> Package: debci
> Version: 1.5
> Severity: normal
> 
> The autopkgtest spec[1] says "Test names are separated by comma and/or
> whitespace" (which I admit is weird, but it's what it says) and the
> reference implementation implements that:
> 
> 
> debian/tests/control in nss-mdns 0.10-8 has space-separated Tests (because
> I misremembered what is allowed) and it looks as though ci.debian.net
> responds to this by only running the first test per stanza:
> https://ci.debian.net/data/packages/unstable/amd64/n/nss-mdns/20170113_063407.autopkgtest.log.gz
> 
> There is no practical impact right now for this particular package,
> because I don't think nss-mdns' tests can be relied on to work in a
> container anyway, so they're all skipped.

debci does not handle debian/tests/control in any way, that is
completely left for autopkgtest. Maybe that is a problem with the
version of autopkgtest that is currently in the workers (4.0.5)?

I plan to upgrade to the latest version from backports soon.


signature.asc
Description: PGP signature

Bug#851232: debci: does not understand space-separated tests

[Simon McVittie]

Package: debci
Version: 1.5
Severity: normal

The autopkgtest spec[1] says "Test names are separated by comma and/or
whitespace" (which I admit is weird, but it's what it says) and the
reference implementation implements that:


debian/tests/control in nss-mdns 0.10-8 has space-separated Tests (because
I misremembered what is allowed) and it looks as though ci.debian.net
responds to this by only running the first test per stanza:
https://ci.debian.net/data/packages/unstable/amd64/n/nss-mdns/20170113_063407.autopkgtest.log.gz

There is no practical impact right now for this particular package,
because I don't think nss-mdns' tests can be relied on to work in a
container anyway, so they're all skipped.

Regards,
S