Bug#704197: Please review: systemd checks
On 2013-04-06 20:44, Michael Stapelberg wrote:
> Hi Niels,
>
> Niels Thykier <niels@thykier.net> writes:
>> I think you are missing a return here?
> Indeed, thanks.
>
> New files are attached, here is the list of things that I know need to
> be fixed:
>
> 1) We don’t have any documentation references in the .desc file yet.
> 2) I need to switch to lab_data_path in check_maintainer_scripts().
>
> Could you please also say a few words on how the usual inclusion process
> works? I.e., what are the next steps after there are no more things left
> to fix?
Tests! Once there are tests for all the new tags (and none of the
existing tests breaks) we are usually ready to accept the checks.
On test writing: We don't have a tutorial for writing tests atm. There
is a bit of help (although a bit outdated) in t/tests/README.
To give you a rough idea of what you are working with, a test case in
t/tests basically follows this pattern:
mkdir -p rundir/test/
rsync t/templates/tests/skel/ rundir/test/
# note t/tests/<testcase>/debian/ is the root of the source package
# thus, t/tests/<testcase>/debian/debian/ is the "debian" dir.
rsync t/tests/<testcase>/debian/ rundir/test/
for file in changelog control ; do
subst rundir/test/debian/${file}.in > rundir/test/debian/${file}
done
cd rundir/test && dpkg-buildpackage -us -uc
lintian -IE rundir/*.changes | sort > actual
diff -u t/tests/<testcase>/tags actual
The test should be named after the check (i.e. systemd-<name> in this
case) and have a sequence of 6000 (in t/tests/<testcase>/desc). A
template for the desc:
Testname: <name of test>
Sequence: 6000
Description: <1-line description>
Test-For:
tag1
tag2
Note: Testname must match the (base)name of the directory[1]. The
description will always be used as synopsis for the package (so it
should follow the same rules). There are more fields you can use (see
t/tests/README).
If you want to test for false-positives, replace Test-For with
Test-Against[2].
The full test suite takes over 30 minutes (user time). Have a look at
Lintian::Tutorial::TestSuite to run only the tests you need while you
work on the tests/check[3]. When you are done, please do a full run of
all tests. Some of the other tests do some funny checks that may
trigger bugs in your check[4].
If the full test suite is too heavy for your machine (my laptop
doesn't like it either), just let me know and I will do the run for you.
> Also, do I need to mark the checks as experimental because they are new?
>
Depends on your level of confidence in your tags and how "well-defined"
the "problem" is[5]. Most tags never had the experimental marker on them.
~Niels
[1] Not checked, but can cause "hard to debug test failures"...
especially if it matches the name of another test (copy-waste ftl).
[2] Test-For requires the test to emit the tag for it to pass.
Test-Against causes it to fail unconditionally if the tag was emitted.
Both fields may be used at the same time.
[3] To test your check for syntax errors (etc.), you may want to run:
debian/rules onlyrun=check-load
After that:
debian/rules onlyrun=<testcase>
debian/rules onlyrun=<check>
debian/rules onlyrun=suite:scripts
When they pass, you might be ready for the full test suite. If you got
cores to spare, throw a DEB_BUILD_OPTIONS=parallel=n.
[4] e.g. suite:debs builds some really interesting debs. The legacy test
"filenames" also got some fun filenames in it.
[5] If there are a lot of corner cases (etc.), the check may need
several iterations to mature.
Reply to: