Bug#692282: [new check] debian/tests/control but not (XS-)Testsuite: autopkgtest header in debian/control
On 2013-01-20 11:39, Nicolas Boulenguez wrote:
> Package: lintian
> Version: 2.5.11
> Followup-For: Bug #692282
>
> Happy new year, and thanks for your advices. Here is a new attempt.
>
Hi,
Thanks for looking into this.
> Adding the Testsuite field in data/common/source-fields causes the
> generated "profiles/debian/main.profile" to change during build. It
> should either be updated in the VCS/source package/whatever at the
> same time, or excluded from lintian source package.
>
Actually, it is the addition of the new check that changes the profile.
But it will only happen "once" (unless you try to undo the change). It
is how we ensure that the profiles are up to date.
> The debian-tests-control-is-not-a-regular-file tag lacks a reference,
> as the current autopkgtest specification does not force the control
> file to be a regular file. Any hint about this issue?
>
We also have tags like "control-file-is-not-a-file" without a reference.
But I guess it is a question of whether that control file is allowed to
be a symlink.
> No check of debian/tests/control contents is done but a trivial one to
> ensure that other ones may eventually be added.
>
Good :)
>
> diff --git a/checks/testsuite b/checks/testsuite
> new file mode 100644
> index 0000000..25f3e3d
> --- /dev/null
> +++ b/checks/testsuite
> @@ -0,0 +1,40 @@
> +# testsuite -- lintian check script -*- perl -*-
> +
Can you please add a copyright/license comment here?
> +package Lintian::testsuite;
> +
> +use strict;
> +use warnings;
> +
> +use Lintian::Tags qw(tag);
> +use Lintian::Util qw(fail file_is_encoded_in_non_utf8);
> +
> +sub run {
> + my ($pkg, $type, $info) = @_;
> + if ($type ne 'source') {
> + fail ('Testsuite check called for binary package.');
> + }
I would probably leave that if-statement out. A lot of things will fail
if checks will be called by the wrong type.
> +
> + my $testsuite = $info->field ('testsuite');
> + my $control = $info->index ('debian/tests/control');
> +
> + if (defined $testsuite xor defined $control) {
> + tag ('inconsistent-testsuite-field');
Style-wise, I prefer not using () with tag. There might be a little bit
of inconsistency here, but I believe the most common use of "tag" is
without ().
> + }
> + if (defined $testsuite and $testsuite ne 'autopkgtest') {
> + tag ('unknown-testsuite', $testsuite);
> + }
> + if (defined $control) {
> + if (not ($info->index ('debian/tests')->is_dir and $control->is_regular_file)) {
^
directories must have a trailing "/" (i.e. 'debian/tests/'). Though, if
'debian/tests/control' is present, then 'debian/tests/' must be a directory.
> + tag ('debian-tests-control-is-not-a-regular-file');
> + } else {
> + my $path = $info->unpacked ($control->name);
> +
> + my $not_utf8_line = file_is_encoded_in_non_utf8 ($path, $type, $pkg);
> + if ($not_utf8_line) {
> + tag ('debian-tests-control-uses-obsolete-national-encoding', "at line $not_utf8_line");
I think "obsolete" suggests that national encoding was once allowed, so
I would probably go without the "obsolete" word in the tag name.
> + }
> + }
> + }
> +}
> +
> +1;
> diff --git a/checks/testsuite.desc b/checks/testsuite.desc
> new file mode 100644
> index 0000000..243a4b8
> --- /dev/null
> +++ b/checks/testsuite.desc
> @@ -0,0 +1,42 @@
> +Check-Script: testsuite
+Author: Nicolas Boulenguez <nicolas@debian.org>
:)
> +Type: source
> +Needs-Info: index, unpacked
> +Info: This script checks the Testsuite field in package dsc files,
> + and debian/tests/control if any.
> +
> +Tag: debian-tests-control-is-not-a-regular-file
> +Severity: wishlist
> +Certainty: certain
> +Info: In case the dsc file contains a Testsuite field, "debian/tests"
> + must be a directory and contain a "control" regular file.
> +# TODO: document this and add a reference here?
> +
> +Tag: debian-tests-control-uses-obsolete-national-encoding
> +Severity: normal
> +Certainty: certain
> +Info: The debian/tests/control file should be valid UTF-8, an encoding
> + of the Unicode character set.
> + .
> + There are many ways to convert a file from an obsoleted encoding like
> + ISO-8859-1; you may for example use "iconv" like:
> + .
> + $ iconv -f ISO-8859-1 -t UTF-8 file > file.new
> + $ mv file.new file
Also here I would drop the obsolete(d)
> +
> +Tag: inconsistent-testsuite-field
> +Severity: wishlist
> +Certainty: certain
> +Info: The package provides a debian/tests/control file but no
> + Testsuite field in the dsc file, or the field exists but not the
> + file.
> + .
> + For discoverability, packages shipping tests for the autopkgtest
> + testing framework should declare their presence in the package
> + description file.
> +Ref: http://anonscm.debian.org/gitweb/?p=autopkgtest/autopkgtest.git;a=blob_plain;f=doc/README.package-tests;hb=HEAD
> +
s/package description file/dsc file/ ? Also, it might be a good idea to
remind people that this can be done by adding
"XS-Testsuite: autopkgtest"
To their d/control file.
> +Tag: unknown-testsuite
> +Severity: normal
> +Certainty: certain
> +Info: Testsuite field in dsc file has a value other than autopkgtest.
> +Ref: http://anonscm.debian.org/gitweb/?p=autopkgtest/autopkgtest.git;a=blob_plain;f=doc/README.package-tests;hb=HEAD
Maybe "The Testsuite field..." ?
> [...]
Once again, thanks for doing this! It is most appreciated. :)
~Niels
Reply to: