Bug#692282: [new check] debian/tests/control but not (XS-)Testsuite: autopkgtest header in debian/control

2013-02-12 Thread Niels Thykier 
On 2013-02-08 18:13, Nicolas Boulenguez wrote:
I wrote at some point:
 Thanks for the code.  I have attached a small diff for some
 stylistic things (note the test .desc changes also prevents a
 synopsis not phrased properly tag).

For the record, the my-changes.diff was intended to be an interdiff
between your changes and the 4-5 lines changes I did (instead of yours
plus mine).  I have attached my-real-changes.diff in case you are
interested.

 Please see test.log.
 
 Sorry, I did not detect these problems because I test with 2.5.11 and
 --root option.
 

It might be help to use Lintian test runner itself to run the test.
Example:

  debian/rules runtests onlyrun=testsuite-control-syntax

Only runs tests named testsuite-control-syntax (I believe we only have
one atm.)  You can also use

  debian/rules runtests onlyrun=testsuite

to run all tests related to the check testsuite[1].

 Three pedantic messages were not reported, I copied similar options
 from another test to t/tests/testsuite-general/desc to unmask them.
 
 Path debian/tests/fifo is reported missing instead of non regular. I
 guess that it will be created once
 t/tests/testsuite-general/pre_build is marked executable.
 

Thanks.

 But I fail to understand the third problem.
 File t/tests/testsuite-general/debian/debian/tests/control
 contains the line Tests: fifo missing-test under_score
 and script checks/testsuite
 reads it with for (split ' ', $paragraph-{'tests'})
 so under_score should be checked after fifo and missing-test.
 The execution order is correct on my machine, but no in your test.log.
 Any clue?

The test runner sorts the output by default as it makes the tests more
robust (otherwise even simple reordering or refactoring would make a
mess of the testsuite).  So, the last issue is that the tags file must
be sorted as well.  I took the liberty of fixing that.  :)

Committed and pushed your addition as 386b337.

Thank you for your contribution, :)
~Niels

[1] Related being starting with testsuite-.

diff -u b/checks/testsuite b/checks/testsuite
--- b/checks/testsuite
+++ b/checks/testsuite
@@ -22,19 +22,12 @@
 use strict;
 use warnings;
 
-use Lintian::Tags qw(
-tag
-);
+use Lintian::Tags qw(tag);
 use Lintian::Util qw(
 file_is_encoded_in_non_utf8
 read_dpkg_control
 );
 
-sub run;
-sub check_control_contents;
-sub check_control_paragraph;
-sub check_test_file;
-
 my @mandatory_fields = qw(
 tests
 );
@@ -77,7 +70,7 @@
 tag 'debian-tests-control-uses-national-encoding', "at line $not_utf8_line";
 }
 
-check_control_contents $info, $path;
+check_control_contents ($info, $path);
 }
 }
 }
@@ -91,7 +84,7 @@
 tag 'syntax-error-in-debian-tests-control', $@;
 } else {
 for (@paragraphs) {
-check_control_paragraph $info, $_;
+check_control_paragraph ($info, $_);
 }
 }
 }
@@ -130,7 +123,7 @@
 $directory = 'debian/tests';
 }
 for (split ' ', $paragraph->{'tests'}) {
-check_test_file $info, $directory, $_;
+check_test_file ($info, $directory, $_);
 }
 }
 }
diff -u b/t/tests/testsuite-control-syntax/desc b/t/tests/testsuite-control-syntax/desc
--- b/t/tests/testsuite-control-syntax/desc
+++ b/t/tests/testsuite-control-syntax/desc
@@ -3,3 +3,3 @@
 Version: 1.0
-Description: Detection of syntax errors in autopkgtest control file.
+Description: Detection of syntax errors in autopkgtest control file
 Test-For: syntax-error-in-debian-tests-control

Bug#692282: [new check] debian/tests/control but not (XS-)Testsuite: autopkgtest header in debian/control

2013-02-08 Thread Niels Thykier 
On 2013-01-25 20:46, Nicolas Boulenguez wrote:
 Package: lintian
 Followup-For: Bug #692282
 
 Checks for syntax errors/unparseable control file. Check that all
 paragraphs have a Tests field. Or even check that all the
 restrictions are known.
 
 Here is a suggestion for review.

Hi,

Thanks for the code.  I have attached a small diff for some stylistic
things (note the test .desc changes also prevents a synopsis not
phrased properly tag).

Secondly, one of the testsuite-tests fails with your changes.  Please
see test.log.

~Niels

commit a336c2cbf8d08e3ba7493e8feaeba1c9f2012ae2
Author: Niels Thykier 
Date:   Fri Feb 8 12:13:13 2013 +0100

stuff

Signed-off-by: Niels Thykier 

diff --git a/checks/testsuite b/checks/testsuite
index db8ced2..e8dd64e 100644
--- a/checks/testsuite
+++ b/checks/testsuite
@@ -23,7 +23,29 @@ use strict;
 use warnings;
 
 use Lintian::Tags qw(tag);
-use Lintian::Util qw(file_is_encoded_in_non_utf8);
+use Lintian::Util qw(
+file_is_encoded_in_non_utf8
+read_dpkg_control
+);
+
+my @mandatory_fields = qw(
+tests
+);
+my %expected_fields = map { $_ => 1 } qw(
+tests
+restrictions
+features
+depends
+tests-directory
+);
+my %expected_features = map { $_ => 1 } qw(
+);
+my %expected_restrictions = map { $_ => 1 } qw(
+breaks-testbed
+build-needed
+needs-root
+rw-build-tree
+);
 
 sub run {
 my ($pkg, $type, $info) = @_;
@@ -47,8 +69,78 @@ sub run {
 if ($not_utf8_line) {
 tag 'debian-tests-control-uses-national-encoding', "at line $not_utf8_line";
 }
+
+check_control_contents ($info, $path);
+}
+}
+}
+sub check_control_contents {
+my ($info, $path) = @_;
+
+my @paragraphs;
+if (not eval { @paragraphs = read_dpkg_control ($path); }) {
+chomp $@;
+$@ =~ s/^syntax error at //;
+tag 'syntax-error-in-debian-tests-control', $@;
+} else {
+for (@paragraphs) {
+check_control_paragraph ($info, $_);
+}
+}
+}
+sub check_control_paragraph {
+my ($info, $paragraph) = @_;
+
+for (@mandatory_fields) {
+if (not exists $paragraph->{$_}) {
+tag 'missing-runtime-tests-field', $_;
+}
+}
+for (keys %$paragraph) {
+if (not exists $expected_fields {$_}) {
+tag 'unknown-runtime-tests-field', $_;
+}
+}
+if (exists $paragraph->{'features'}) {
+for (split ' ', $paragraph->{'features'}) {
+if (not exists $expected_features {$_}) {
+tag 'unknown-runtime-tests-feature', $_;
+}
 }
 }
+if (exists $paragraph->{'restrictions'}) {
+for (split ' ', $paragraph->{'restrictions'}) {
+if (not exists $expected_restrictions {$_}) {
+tag 'unknown-runtime-tests-restriction', $_;
+}
+}
+}
+if (exists $paragraph->{'tests'}) {
+my $directory;
+if (exists $paragraph->{'tests-directory'}) {
+$directory = $paragraph->{'tests-directory'};
+} else {
+$directory = 'debian/tests';
+}
+for (split ' ', $paragraph->{'tests'}) {
+check_test_file ($info, $directory, $_);
+}
+}
+}
+sub check_test_file {
+my ($info, $directory, $name) = @_;
+
+if ($name !~ /^[[:digit:][:lower:]\+\-\.\/]*$/) {
+tag 'illegal-runtime-test-name', $name;
+}
+my $path = "$directory/$name";
+my $index = $info->index ($path);
+if (not defined $index) {
+tag 'missing-runtime-test-file', $path;
+} elsif (not $index->is_regular_file) {
+tag 'runtime-test-file-is-not-a-regular-file', $path;
+}
+# Test files are allowed not to be executable.
 }
 
 1;
diff --git a/checks/testsuite.desc b/checks/testsuite.desc
index 0210fae..cf9c3f7 100644
--- a/checks/testsuite.desc
+++ b/checks/testsuite.desc
@@ -23,6 +23,14 @@ Info: The debian/tests/control file should be valid UTF-8, an encoding
   $ iconv -f ISO-8859-1 -t UTF-8 file  file.new
   $ mv file.new file
 
+Tag: illegal-runtime-test-name
+Severity: normal
+Certainty: certain
+Info: Runtime test names in debian/tests/control are only allowed to
+ contain decimal digits, lowercase ASCII letters, plus or minus signs,
+ dots or slashes.
+Ref: http://anonscm.debian.org/gitweb/?p=autopkgtest/autopkgtest.git;a=blob_plain;f=doc/README.package-tests;hb=HEAD
+
 Tag: inconsistent-testsuite-field
 Severity: wishlist
 Certainty: certain
@@ -35,6 +43,61 @@ Info: The package provides a debian/tests/control file but no
  dsc file by adding "XS-Testsuite: autopkgtest" to their debian/control.
 Ref: http://anonscm.debian.org/gitweb/?p=autopkgtest/autopkgtest.git;a=blob_plain;f=doc/README.package-tests;hb=HEAD
 
+Tag: missing-runtime-tests-field
+Severity: normal
+Certainty: certain
+Info: A mandatory field is missing in some paragraph of the
+

Bug#692282: [new check] debian/tests/control but not (XS-)Testsuite: autopkgtest header in debian/control

2013-02-08 Thread Nicolas Boulenguez 
 Please see test.log.

Sorry, I did not detect these problems because I test with 2.5.11 and
--root option.

Three pedantic messages were not reported, I copied similar options
from another test to t/tests/testsuite-general/desc to unmask them.

Path debian/tests/fifo is reported missing instead of non regular. I
guess that it will be created once
t/tests/testsuite-general/pre_build is marked executable.

But I fail to understand the third problem.
File t/tests/testsuite-general/debian/debian/tests/control
contains the line Tests: fifo missing-test under_score
and script checks/testsuite
reads it with for (split ' ', $paragraph-{'tests'})
so under_score should be checked after fifo and missing-test.
The execution order is correct on my machine, but no in your test.log.
Any clue?
diff --git a/t/tests/testsuite-general/desc b/t/tests/testsuite-general/desc
index 31f80b7..1f12cae 100644
--- a/t/tests/testsuite-general/desc
+++ b/t/tests/testsuite-general/desc
@@ -2,6 +2,8 @@ Testname: testsuite-general
 Sequence: 6000
 Version: 1.0
 Description: General tests of the autopkgtest control file
+Options: -I -E --pedantic
+Skeleton: pedantic
 Test-For:
  debian-tests-control-uses-national-encoding
  illegal-runtime-test-name
diff --git a/t/tests/testsuite-general/pre_build b/t/tests/testsuite-general/pre_build
old mode 100644
new mode 100755

Bug#692282: [new check] debian/tests/control but not (XS-)Testsuite: autopkgtest header in debian/control

2013-01-25 Thread Nicolas Boulenguez 
Package: lintian
Followup-For: Bug #692282

 Checks for syntax errors/unparseable control file. Check that all
 paragraphs have a Tests field. Or even check that all the
 restrictions are known.

Here is a suggestion for review.
From 0e6b9a16707dab1d32ee772da0b3ed81e2907a49 Mon Sep 17 00:00:00 2001
From: Nicolas Boulenguez nico...@debian.org
Date: Fri, 25 Jan 2013 20:25:51 +0100
Subject: * Check debian/tests/control fields if autopkgtest is used.

---
 checks/testsuite   |  103 +++-
 checks/testsuite.desc  |   63 
 .../debian/debian/control.in   |   16 +++
 .../debian/debian/tests/control|5 +
 t/tests/testsuite-control-syntax/desc  |5 +
 t/tests/testsuite-control-syntax/tags  |1 +
 .../testsuite-general/debian/debian/tests/control  |   16 +++
 .../debian/debian/tests/under_score|2 +
 .../testsuite-general/debian/subdir/test-in-subdir |2 +
 t/tests/testsuite-general/desc |7 ++
 t/tests/testsuite-general/pre_build|6 ++
 t/tests/testsuite-general/tags |7 ++
 12 files changed, 231 insertions(+), 2 deletions(-)
 create mode 100644 t/tests/testsuite-control-syntax/debian/debian/control.in
 create mode 100644 t/tests/testsuite-control-syntax/debian/debian/tests/control
 create mode 100644 t/tests/testsuite-control-syntax/desc
 create mode 100644 t/tests/testsuite-control-syntax/tags
 create mode 100644 t/tests/testsuite-general/debian/debian/tests/under_score
 create mode 100644 t/tests/testsuite-general/debian/subdir/test-in-subdir
 create mode 100644 t/tests/testsuite-general/pre_build

diff --git a/checks/testsuite b/checks/testsuite
index db8ced2..bf7cf74 100644
--- a/checks/testsuite
+++ b/checks/testsuite
@@ -22,8 +22,37 @@ package Lintian::testsuite;
 use strict;
 use warnings;
 
-use Lintian::Tags qw(tag);
-use Lintian::Util qw(file_is_encoded_in_non_utf8);
+use Lintian::Tags qw(
+tag
+);
+use Lintian::Util qw(
+file_is_encoded_in_non_utf8
+read_dpkg_control
+);
+
+sub run;
+sub check_control_contents;
+sub check_control_paragraph;
+sub check_test_file;
+
+my @mandatory_fields = qw(
+tests
+);
+my %expected_fields = map { $_ = 1 } qw(
+tests
+restrictions
+features
+depends
+tests-directory
+);
+my %expected_features = map { $_ = 1 } qw(
+);
+my %expected_restrictions = map { $_ = 1 } qw(
+breaks-testbed
+build-needed
+needs-root
+rw-build-tree
+);
 
 sub run {
 my ($pkg, $type, $info) = @_;
@@ -47,8 +76,78 @@ sub run {
 if ($not_utf8_line) {
 tag 'debian-tests-control-uses-national-encoding', at line $not_utf8_line;
 }
+
+check_control_contents $info, $path;
+}
+}
+}
+sub check_control_contents {
+my ($info, $path) = @_;
+
+my @paragraphs;
+if (not eval { @paragraphs = read_dpkg_control ($path); }) {
+chomp $@;
+$@ =~ s/^syntax error at //;
+tag 'syntax-error-in-debian-tests-control', $@;
+} else {
+for (@paragraphs) {
+check_control_paragraph $info, $_;
 }
 }
 }
+sub check_control_paragraph {
+my ($info, $paragraph) = @_;
+
+for (@mandatory_fields) {
+if (not exists $paragraph-{$_}) {
+tag 'missing-runtime-tests-field', $_;
+}
+}
+for (keys %$paragraph) {
+if (not exists $expected_fields {$_}) {
+tag 'unknown-runtime-tests-field', $_;
+}
+}
+if (exists $paragraph-{'features'}) {
+for (split ' ', $paragraph-{'features'}) {
+if (not exists $expected_features {$_}) {
+tag 'unknown-runtime-tests-feature', $_;
+}
+}
+}
+if (exists $paragraph-{'restrictions'}) {
+for (split ' ', $paragraph-{'restrictions'}) {
+if (not exists $expected_restrictions {$_}) {
+tag 'unknown-runtime-tests-restriction', $_;
+}
+}
+}
+if (exists $paragraph-{'tests'}) {
+my $directory;
+if (exists $paragraph-{'tests-directory'}) {
+$directory = $paragraph-{'tests-directory'};
+} else {
+$directory = 'debian/tests';
+}
+for (split ' ', $paragraph-{'tests'}) {
+check_test_file $info, $directory, $_;
+}
+}
+}
+sub check_test_file {
+my ($info, $directory, $name) = @_;
+
+if ($name !~ /^[[:digit:][:lower:]\+\-\.\/]*$/) {
+tag 'illegal-runtime-test-name', $name;
+}
+my $path = $directory/$name;
+my $index = $info-index ($path);
+if (not defined $index) {
+tag 'missing-runtime-test-file', $path;
+} elsif (not $index-is_regular_file) {
+tag 'runtime-test-file-is-not-a-regular-file', $path;
+}
+# Test files are allowed not to be executable.
+}
 
 1;

Bug#692282: [new check] debian/tests/control but not (XS-)Testsuite: autopkgtest header in debian/control

2013-01-22 Thread Niels Thykier 
Control: tags -1 pending

On 2013-01-21 00:46, Nicolas Boulenguez wrote:
 Package: lintian
 Followup-For: Bug #692282
 
   if (not ($info-index ('debian/tests')-is_dir and 
   $control-is_regular_file)) {
  directories must have a trailing / (i.e. 'debian/tests/').
 Not sure to understand what you mean by must.
 On my machine, $info-index ('debian/tests')-is_dir returns true
 when the directory exists, as I expected from the documentation.
 
 
 [...]


Caused by a double mistake actually.  First, the documentation was not
clear in this point and secondly, the index code for souce packages did
not add the trailing slash as expected.  Both should be fixed now with
commits:

 * 0e9631839fe574d38826392c59850c0dc58300aa
 * 6dde19562b1283446da55469d0af34ae9658731f
 * 38df05466e28ab6d08ef2e8f822f9bc570844b2e

Anyway, merged your patch as 4fab7f84ba8d17f91f9e79a00a6b6e7272608e99
with some tests to go with it.

In case you are interested in extending the check, possible additional
checks could include checks for syntax errors/unparseable control file
(checks/source-copyright has a similar code piece for that).  Check that
all paragraphs have a Tests field.  Or even check that all the
restrictions are known.

Thanks for the check,
~Niels


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org

Bug#692282: [new check] debian/tests/control but not (XS-)Testsuite: autopkgtest header in debian/control

2013-01-20 Thread Nicolas Boulenguez 
Package: lintian
Version: 2.5.11
Followup-For: Bug #692282

Happy new year, and thanks for your advices. Here is a new attempt.

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.

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?

No check of debian/tests/control contents is done but a trivial one to
ensure that other ones may eventually be added.
diff --git a/checks/testsuite b/checks/testsuite
new file mode 100644
index 000..25f3e3d
--- /dev/null
+++ b/checks/testsuite
@@ -0,0 +1,40 @@
+# testsuite -- lintian check script -*- perl -*-
+
+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.');
+}
+
+my $testsuite = $info-field ('testsuite');
+my $control = $info-index ('debian/tests/control');
+
+if (defined $testsuite xor defined $control) {
+tag ('inconsistent-testsuite-field');
+}
+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)) {
+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);
+}
+}
+}
+}
+
+1;
diff --git a/checks/testsuite.desc b/checks/testsuite.desc
new file mode 100644
index 000..243a4b8
--- /dev/null
+++ b/checks/testsuite.desc
@@ -0,0 +1,42 @@
+Check-Script: testsuite
+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 gt; file.new
+  $ mv file.new file
+
+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
+
+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
diff --git a/data/common/source-fields b/data/common/source-fields
index 17ed60c..334a1be 100644
--- a/data/common/source-fields
+++ b/data/common/source-fields
@@ -23,6 +23,7 @@ python-version
 ruby-versions
 source
 standards-version
+testsuite
 uploaders
 vcs-arch
 vcs-browser

Bug#692282: [new check] debian/tests/control but not (XS-)Testsuite: autopkgtest header in debian/control

2013-01-20 Thread Niels Thykier 
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 000..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 000..243a4b8
 --- /dev/null
 +++ b/checks/testsuite.desc
 @@ -0,0 +1,42 @@
 +Check-Script: testsuite
  +Author: Nicolas Boulenguez nico...@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 gt; 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:

Bug#692282: [new check] debian/tests/control but not (XS-)Testsuite: autopkgtest header in debian/control

2013-01-20 Thread Nicolas Boulenguez 
Package: lintian
Followup-For: Bug #692282

  if (not ($info-index ('debian/tests')-is_dir and 
  $control-is_regular_file)) {

 directories must have a trailing / (i.e. 'debian/tests/').

Not sure to understand what you mean by must.
On my machine, $info-index ('debian/tests')-is_dir returns true
when the directory exists, as I expected from the documentation.
diff --git a/checks/testsuite b/checks/testsuite
index 25f3e3d..db8ced2 100644
--- a/checks/testsuite
+++ b/checks/testsuite
@@ -1,37 +1,51 @@
 # testsuite -- lintian check script -*- perl -*-
 
+# Copyright (C) 2013 Nicolas Boulenguez nico...@debian.org
+
+# This file is part of lintian.
+
+# Lintian is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# Lintian is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with Lintian.  If not, see http://www.gnu.org/licenses/.
+
 package Lintian::testsuite;
 
 use strict;
 use warnings;
 
 use Lintian::Tags qw(tag);
-use Lintian::Util qw(fail file_is_encoded_in_non_utf8);
+use Lintian::Util qw(file_is_encoded_in_non_utf8);
 
 sub run {
 my ($pkg, $type, $info) = @_;
-if ($type ne 'source') {
-fail ('Testsuite check called for binary package.');
-}
 
 my $testsuite = $info-field ('testsuite');
 my $control = $info-index ('debian/tests/control');
 
 if (defined $testsuite xor defined $control) {
-tag ('inconsistent-testsuite-field');
+tag 'inconsistent-testsuite-field';
 }
 if (defined $testsuite and $testsuite ne 'autopkgtest') {
-tag ('unknown-testsuite', $testsuite);
+tag 'unknown-testsuite', $testsuite;
 }
 if (defined $control) {
-if (not ($info-index ('debian/tests')-is_dir and $control-is_regular_file)) {
-tag ('debian-tests-control-is-not-a-regular-file');
+if (not $control-is_regular_file) {
+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);
+tag 'debian-tests-control-uses-national-encoding', at line $not_utf8_line;
 }
 }
 }
diff --git a/checks/testsuite.desc b/checks/testsuite.desc
index 243a4b8..0210fae 100644
--- a/checks/testsuite.desc
+++ b/checks/testsuite.desc
@@ -1,4 +1,5 @@
 Check-Script: testsuite
+Author: Nicolas Boulenguez nico...@debian.org
 Type: source
 Needs-Info: index, unpacked
 Info: This script checks the Testsuite field in package dsc files,
@@ -9,15 +10,14 @@ 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
+Tag: debian-tests-control-uses-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
+ There are many ways to convert a file from an encoding like
  ISO-8859-1; you may for example use iconv like:
  .
   $ iconv -f ISO-8859-1 -t UTF-8 file gt; file.new
@@ -32,11 +32,13 @@ Info: The package provides a debian/tests/control file but no
  .
  For discoverability, packages shipping tests for the autopkgtest
  testing framework should declare their presence in the package
- description file.
+ dsc file by adding XS-Testsuite: autopkgtest to their debian/control.
 Ref: http://anonscm.debian.org/gitweb/?p=autopkgtest/autopkgtest.git;a=blob_plain;f=doc/README.package-tests;hb=HEAD
 
 Tag: unknown-testsuite
 Severity: normal
 Certainty: certain
-Info: Testsuite field in dsc file has a value other than autopkgtest.
+Info: The dsc file sets Testsuite to a value other than autopkgtest,
+ the only one allowed. This field is most probably copied by
+ dpkg-source from XS-Testsuite in debian/control.
 Ref: http://anonscm.debian.org/gitweb/?p=autopkgtest/autopkgtest.git;a=blob_plain;f=doc/README.package-tests;hb=HEAD
diff --git a/checks/testsuite b/checks/testsuite
new file mode 100644
index 000..db8ced2
--- /dev/null
+++ b/checks/testsuite
@@ -0,0 +1,54 @@
+# testsuite -- lintian check script -*- perl -*-
+
+# Copyright (C) 2013 Nicolas Boulenguez nico...@debian.org
+
+# This file is part of lintian.

Bug#692282: [new check] debian/tests/control but not (XS-)Testsuite: autopkgtest header in debian/control

2013-01-07 Thread Christoph Berg 
Re: Stefano Zacchiroli 2012-11-04 
20121104170013.30086.35818.reportbug@usha.takhisis.invalid
 Can you please add a lintian test that will warn if a debian/tests/control 
 file
 exists, but no XS-Testsuite: autopkgtest header is present in the source
 stanza of debian/control ?

And of course, the generic warning about unknown fields should be
dropped:

W: pgbouncer source: unknown-field-in-dsc testsuite

Christoph
-- 
c...@df7cb.de | http://www.df7cb.de/


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org

Bug#692282: [new check] debian/tests/control but not (XS-)Testsuite: autopkgtest header in debian/control

2013-01-07 Thread Christoph Berg 
Re: To Stefano Zacchiroli 2013-01-07 20130107155757.ga28...@msgid.df7cb.de
 Re: Stefano Zacchiroli 2012-11-04 
 20121104170013.30086.35818.reportbug@usha.takhisis.invalid
  Can you please add a lintian test that will warn if a debian/tests/control 
  file
  exists, but no XS-Testsuite: autopkgtest header is present in the source
  stanza of debian/control ?
 
 And of course, the generic warning about unknown fields should be
 dropped:
 
 W: pgbouncer source: unknown-field-in-dsc testsuite

Another thought: There should still be a warning if the value of the
field isn't autopkgtest.

Christoph
-- 
c...@df7cb.de | http://www.df7cb.de/


--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org

Bug#692282: [new check] debian/tests/control but not (XS-)Testsuite: autopkgtest header in debian/control

2013-01-07 Thread Niels Thykier 
On 2012-11-23 23:42, Nicolas Boulenguez wrote:
 Package: lintian
 Version: 2.5.10.2
 Tags: patch
 Followup-For: Bug #692282
 
 Please consider the attached patch.
 

Hi,

Thanks for looking at writing at a patch and sorry for the delay in
getting back to you.

I hope I can convince you to do a couple of minor changes to your
original patch.  As Christoph mentioned, the value of the header should
be autopkgtest.
  Secondly, it would probably be a good idea to move this to
checks/fields and use $info-field ('testsuite') instead of
$info-source_field ('xs-testsuite')[1].  Note that checks/fields is
also applied to binary packages, so a if ($type eq 'source') is needed.

 
 lintian.diff
 
 
 diff -rNu ../old/lintian-2.5.10.2/checks/control-file ./checks/control-file
 --- ../old/lintian-2.5.10.2/checks/control-file   2012-09-17 
 11:56:05.0 +0200
 +++ ./checks/control-file 2012-11-23 23:31:07.0 +0100
 @@ -265,6 +265,10 @@
  }
  }
  
 +if (defined $info-source_field ('xs-testsuite')
 +xor -e $info-debfiles('tests/control')) {
   ^^^

This has a minor issue in that tests or tests/control could be a symlink
pointing to some system file, which could be used to disclose the
existance of a given file on the system running Lintian.

 +tag 'inconsistent-xs-testsuite-field'; }
 +
  }
  
  
 diff -rNu ../old/lintian-2.5.10.2/checks/control-file.desc 
 ./checks/control-file.desc
 --- ../old/lintian-2.5.10.2/checks/control-file.desc  2012-09-17 
 11:56:05.0 +0200
 +++ ./checks/control-file.desc2012-11-23 22:13:12.0 +0100
 @@ -200,3 +200,15 @@
  Info: The control file contains commented-out VCS-* lines, most
   probably a result of dh_make. These URLs should either be valid and
   uncommented, or removed.
 +
 +Tag: inconsistent-xs-testsuite-field
 +Severity: wishlist
 +Certainty: certain
 +Ref: 
 http://anonscm.debian.org/gitweb/?p=autopkgtest/autopkgtest.git;a=blob_plain;f=doc/README.package-tests;hb=HEAD
 +Info: A debian/tests/control file exists, but no XS-Testsuite: autopkgtest
 + header is present in the source stanza of debian/control, or the header
 + exists but not the file.
 + .
 + For discoverability, packages shipping tests for the autopkgtest
 + testing framework should declare their presence using an
 + XS-Testsuite: autopkgtest header in debian/control.

A good description, though assuming the migration to $info-field the
tag name should probably change and the description would need a few
minor adjustments (s/XS-// s,debian/control,.dsc file, etc.)

The alternative to moving this to checks/fields could be to make a
dedicated testsuite-check, that would also do checking of the tests
control file(s).

~Niels

[1] Rationale for:
 * using $info-field ('testsuite')
   - It tests the field in the .dsc file rather than the d/control.  If
 this field is added automatically by some tool (e.g. dpkg-source)
 in the future, the check will still produce the right result
 * moving it to checks/fields
   - It is where most of our checks for .dsc fields are.


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org

Bug#692282: [new check] debian/tests/control but not (XS-)Testsuite: autopkgtest header in debian/control

2012-11-23 Thread Nicolas Boulenguez 
Package: lintian
Version: 2.5.10.2
Tags: patch
Followup-For: Bug #692282

Please consider the attached patch.
diff -rNu ../old/lintian-2.5.10.2/checks/control-file ./checks/control-file
--- ../old/lintian-2.5.10.2/checks/control-file	2012-09-17 11:56:05.0 +0200
+++ ./checks/control-file	2012-11-23 23:31:07.0 +0100
@@ -265,6 +265,10 @@
 }
 }
 
+if (defined $info-source_field ('xs-testsuite')
+xor -e $info-debfiles('tests/control')) {
+tag 'inconsistent-xs-testsuite-field'; }
+
 }
 
 
diff -rNu ../old/lintian-2.5.10.2/checks/control-file.desc ./checks/control-file.desc
--- ../old/lintian-2.5.10.2/checks/control-file.desc	2012-09-17 11:56:05.0 +0200
+++ ./checks/control-file.desc	2012-11-23 22:13:12.0 +0100
@@ -200,3 +200,15 @@
 Info: The control file contains commented-out VCS-* lines, most
  probably a result of dh_make. These URLs should either be valid and
  uncommented, or removed.
+
+Tag: inconsistent-xs-testsuite-field
+Severity: wishlist
+Certainty: certain
+Ref: http://anonscm.debian.org/gitweb/?p=autopkgtest/autopkgtest.git;a=blob_plain;f=doc/README.package-tests;hb=HEAD
+Info: A debian/tests/control file exists, but no XS-Testsuite: autopkgtest
+ header is present in the source stanza of debian/control, or the header
+ exists but not the file.
+ .
+ For discoverability, packages shipping tests for the autopkgtest
+ testing framework should declare their presence using an
+ XS-Testsuite: autopkgtest header in debian/control.

Bug#692282: [new check] debian/tests/control but not (XS-)Testsuite: autopkgtest header in debian/control

2012-11-04 Thread Stefano Zacchiroli 
Package: lintian
Version: 2.5.10.2
Severity: wishlist

[ context is https://lists.debian.org/debian-qa/2012/11/msg9.html where we
  are discussing the integration of autopkgtest runs with the new
  jenkins.debian.net service ]

For discoverability, packages shipping tests for the autopkgtest testing
framework [1] should declare their presence using an XS-Testsuite:
autopkgtest header in debian/control (see last section of [2]).

At present, only 3 packages in sid/main have that header [3], whereas 68
packages have a debian/tests/control file [4].

Can you please add a lintian test that will warn if a debian/tests/control file
exists, but no XS-Testsuite: autopkgtest header is present in the source
stanza of debian/control ?

I'll propose a wishlist mass-bug-filing to fix the packages already in the
archive, but the test would be nice to both avoid the problem in the future,
and more easily quantify its presence in the archive.


Thanks for maintaining lintian!
Cheers.


[1]: http://packages.qa.debian.org/a/autopkgtest.html
[2]: 
http://anonscm.debian.org/gitweb/?p=autopkgtest/autopkgtest.git;a=blob_plain;f=doc/README.package-tests;hb=HEAD
[3]: see attached autopkgtest-list.header.txt , obtained starting from
 $ wget -q 
http://http.debian.net/debian/dists/unstable/main/source/Sources.bz2 -O- | 
bunzip2 -c | grep-dctrl -s Package -F Testsuite autopkgtest -
[4]: see attached autopkgtest-list.content.txt , obtained starting from
 $ wget -q 
http://http.debian.net/debian/dists/unstable/main/Contents-source.gz -O- | 
zgrep -m1 -E '^debian/tests/control\s'

-- System Information:
Debian Release: wheezy/sid
  APT prefers unstable
  APT policy: (500, 'unstable'), (500, 'testing'), (500, 'stable'), (1, 
'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 3.2.0-4-amd64 (SMP w/4 CPU cores)
Locale: LANG=it_IT.UTF-8, LC_CTYPE=it_IT.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash

Versions of packages lintian depends on:
ii  binutils   2.22-7.1
ii  bzip2  1.0.6-4
ii  diffstat   1.55-3
ii  file   5.11-2
ii  gettext0.18.1.1-9
ii  hardening-includes 2.2
ii  intltool-debian0.35.0+20060710.1
ii  libapt-pkg-perl0.1.26+b1
ii  libarchive-zip-perl1.30-6
ii  libc-bin   2.13-36
ii  libclass-accessor-perl 0.34-1
ii  libclone-perl  0.31-1+b2
ii  libdpkg-perl   1.16.9
ii  libemail-valid-perl0.190-1
ii  libipc-run-perl0.92-1
ii  libparse-debianchangelog-perl  1.2.0-1
ii  libtimedate-perl   1.2000-1
ii  liburi-perl1.60-1
ii  locales2.13-36
ii  locales-all [locales]  2.13-36
ii  man-db 2.6.3-1
ii  patchutils 0.3.2-1.1
ii  perl [libdigest-sha-perl]  5.14.2-14

lintian recommends no packages.

Versions of packages lintian suggests:
pn  binutils-multiarch none
ii  dpkg-dev   1.16.9
ii  libhtml-parser-perl3.69-2
pn  libperlio-gzip-perlnone
ii  libtext-template-perl  1.45-2
ii  man-db 2.6.3-1
ii  xz-utils [lzma]5.1.1alpha+20120614-1

-- no debconf information
postgresql-common
tabix
upower
apipkg
bobo
bzr
bzr-email
bzr-fastimport
bzr-git
bzr-loom
bzr-rewrite
bzr-stats
bzr-svn
bzr-upload
execnet
gamera
libgmpada
libgtkada
libncursesada
libtexttools
mafft
mawk
ocrad
postgresql-common
pstreams
pytest-xdist
python-byteplay
python-chameleon
python-fastimport
python-mechanize
rake
ruby-switch
samba4
sinntp
sourcecodegen
sphinx
tabix
transaction
udisks
upower
zc.buildout
zc.lockfile
zconfig
zdaemon
zodb
zope.authentication
zope.browser
zope.cachedescriptors
zope.component
zope.configuration
zope.contenttype
zope.copy
zope.deprecation
zope.dottedname
zope.event
zope.exceptions
zope.hookable
zope.i18n
zope.i18nmessageid
zope.interface
zope.location
zope.proxy
zope.publisher
zope.schema
zope.security
zope.sendmail
zope.sqlalchemy
zope.testbrowser
zope.testing
zope.testrunner
zope.traversing