Lintian test suite proposal
Hi Felix,
Context for lint-maint: Felix Lechner sent me a proposed change for the
test suite and asked me for write an email with the feedback. I thought
it was useful to include lint-maint because it affects all contributors
of lintian (if implemented). The proposal is quoted below interleaved
with my comments and is to be an update/edit to t/tests/README.
Thanks for working on improving Lintian. I should once again repeat
that my work on lintian has dwindled and I may not be the right
contributor any longer to decide which way we are going with the test
suite (IOW, take my review with a grain of salt).
As I read it, there are two orthogonal but interesting ideas.
1. The tags.d layout
2. The Abbreviated-Tags proposal pigging backed on top of tags.d.
As I read it, these proposals should be feasible implement separately
(i.e. Abbreviated-Tags could be retrofitted into the existing framework
without adding tags.d). This is mostly relevant if one of the proposals
turn out to be controversial and the other is not - in that case, we can
move forward with the uncontroversial one.
Personally, I feel I understand Abbreviated-Tags good enough that I can
say I would support it and I would recommend it was made the default for
new tests (modulo a few minor remarks).
On the tags.d layout, I can mostly see where you would like to go but
it is not clear to me when the test runner will accept vs. fail a test.
> New tags.d layout
> -----------------
General remarks about the text (assuming it is for t/tests/README):
* I think we should avoid the "New" / "Old" in this document if it is
an addition to t/tests/README. The problem with using "new" for
everything is that in a year it will no longer be new and at some
point it could even be superseded by a different format/layout as we
improve ourselves.
* The text sometimes stops being a README document and starts being
sales-pitch and then jumps back to being a README a bit later only
to become a TODO list in the end. I have highlighted a few of these
but I stopped when I realised it was a general problem.
- Note there is nothing wrong with the sales-pitch/rationale for the
proposals. The "issue" is that I was told to read this as a
t/tests/README change and some of the text need tweaking to fit
there.
>
> In addition to the established format described in the remainder of
> this document, our test suite now supports a new directory-based
> format for expected tags.
>
> The new format no longer compares the entire lintian output. Instead,
> it uses separate files for each lintian check. The files are located
> in a directory called tags.d.
>> Each file is named after a lintian check. It contains all tags issued
> by a particular check. The tags can be in any order. Tags from checks
> are ignored and will no longer interfere.
The last sentence is a bit unclear to me. I suspect you mean that tags
are ignored if they do not belong to one of the checks in tags.d?
Assuming there is tags.d/X file and it contains the tag "Y". Will the
test fail if the check X emits the tag "Z" in addition to "Y"?
> This decoupling has several
> advantages. For example, one can now enable '--pedantic' without
> having to deal with any noise from other checks. Instead, one can
> conveniently test just the tags expected from a particular check.
>
> The tags.d directory also contains a file named 'options' which gives
> the user a way to restore the old behavior.
How come you are going for a new "options" file for these options
instead of adding them to the per-test desc file?
> When set, 'Exhaustive-Set:
> yes" will cause any extra tags in the output to fail the test. Since
> checks generally have little correlation, this option will not be
> useful in many cases. For most tests, the default setting of
> 'Exhaustive-Set: no' will be a great relief.
I could do with some examples of when the test succeeds and fails given
a given tags.d folder/content, different Exhaustive-Set values and
different lintian output.
> The lintian option
> '--pedantic' might even become the default setting for the test suite
> going forward.
>
The last sentence (about making --pedantic default in tests) makes sense
in the proposal as it is part of the reason why this proposal might be
useful. However, I do not think it would belong in t/tests/README.
> For more flexibility, there is another option 'Abbreviated-Tags: yes'.
> It allows expected tags to be listed in a shorter and possibly
> more convenient format. First, the letter code at the beginning of
> each line is dropped and reconstructed automatically. Tags are only
> ever issued with one of these letters, so the chance of an internal
> error is small. Instead of worrying about a tag's severity and
> certainty, you can focus on which tags are being issued and why.
>
Note that AFAICT, the Abbreviated-Tags could be added to the existing
framework without the new layout (after resolving the feature
interaction with e.g. "Sort: yes").
> An example for the abbreviated format is:
>
> - File: tags.d/changelog-file.tags
>
> : epoch-changed-but-upstream-version-did-not-go-backwards 3 >= 0.0.1
> : no-upstream-changelog
> : epoch-change-without-comment (none) -> 2
>
> Please note the colons at the beginning of each line. They are necessary
> to distinguish plain lintian tags from other suites, as in:
>
> - File: tags.d/watch-file.tags
>
> source: debian-watch-does-not-check-gpg-signature
>
Would it make sense to also let the test runner reconstruct the
"source:"-bit (etc.) such that the abbreviated format will be:
"""
<tag1>
<tag2> <extra>
<tag3>
"""
At least, it seems more natural to me that if the test runner is going
to assist us, it might as well assist us with all the "boring" parts of
the output.
> The changes in the new format work together to dissociate checks, tags
> and tests. One goal is to make everything easier to refactor in the
> future.
>
> There are many other benefits, as well: With the new format, we
> automatically confirm for each check that no tags other than those
> specified were issued. That obsoletes the option Test-Against.
>
Test-Against is an interesting relic. Test-Against has 3 purposes:
1) What you describe, where it is now completely and utterly obsolete
because the test would fail before Test-Against is validated.
(unless you consider it a "stop gap" to avoid people blindly shoving
a false-positive regression in /tags)
2) For calculating t/COVERAGE which you mention later
3) For selecting all tests that are relevant to a given tag.
I believe the proposal covers 1+2. But I do not see 3 mentioned in the
proposal. I am fine with dropping Test-Against if item 3 is no longer a
useful feature but I figured we ought to be explicit about it.
> When a check is expected to run without issuing tags, please use a
> file with one or more empty lines. (Git won't commit files that are
> totally empty.)
Note: We have plenty of completely empty files committed in git; what
git refuses to track are empty directories.
> Since we now know the names of all checks in a test,
> this also obsoletes the option Test-For.
>
True, but this could also be retrofitted to the current solution with
Test-For being optional when it is 1:1 with the tags file. That would
remove the vast majority of Test-For entries.
Also, I would recommend avoid using first person ("we" or "I") in the
README. It is fine for an informal description or a proposal.
> Neither Test-For or Test-Against is needed going forward. They are two
> fewer things to worry about. Both fields were previously used to
> calculate the COVERAGE. For tests in the new format, the contribution
> to coverage is calculated from other data. With fewer opportunities
> for human error (for example by forgetting to update Test-For), the
> COVERAGE information will be at least as accurate as before. The
> coverage may also be wider due to the automatic Test-Against, as
> explained above. In any event, the COVERAGE information continues to
> be useful and up-to-date.
>
I like this aspect of the proposal aiming to reduce redundant
information. :)
> Now you may wonder, does it have any drawbacks? The answer is
> yes.
A bit to informal / speech-like for the README.
> Since unexpected tags are generally ignored, it can be hard to
> tell what a test could also be good for. It may be best practice to
> use only narrow tests, but you can get additional recommendations for
> wider unit testing by running 'runtests' in verbose mode. It will tell
> you which other checks emit tags, so you can add them if you like.
>
> As a special rule, 'runtests' will expect absolutely zero lintian
> output when no tag files are present. In that case, the option
> 'Exhaustive-Set' has no effect. This eliminates the absurd and
> unwanted possibility that no testing takes place. The resulting error
> messages also make it easier to write unit tests for particular
> features, one test at a time.
>
> The features described presently only work with regular lintian
> output. For XML or colon-separated output, please continue to use the
> regular, uniform 'tags' files.
>
> There are few other changes: The older format used a script
> 'test_calibration' to adjust expected tags for corner cases (mainly
> for release architectures). Such scripts now operate on a per-check
> basis. They have names like tags.d/${check}.calibrate. Please note
> that the input and output formats are the same as for the expected
> tags. If you use abbreviated tags in your test, please make sure your
> calibration script issues them, too. The scripts get only a filtered
> lintian output that relates to their particular check.
>
> A few tests examine lintian's debug or non-tag output. Those lines
> are now found in tags.d/messages. You can use a calibration script
> with them, but there is no abbreviated format.
>
How is lintian's message output matched with tags.d/messages? Are both
sorted first or ...?
> Since the expected tags can now be listed in any order, the Sort
> option is probably obsolete, unless a calibration script depends on
> it.
>
I think the Abbreviated-Tags proposal will overshadow the Sort option
for tags. As such, Sort will generally only be useful for
"Abbreviated-Tags: no" tests (but I would not be surprised if most of
them were also "Sort: no" tests):
I would not keep Sort around for calibration scripts; it is trivial to
invoke sort in such a script.
> The majority of tests were converted automatically. Tags previously
> listed in one large 'tags' file are now separated into several smaller
> files that group tags emitted by the same check. For tags found in
> Test-Against, we made sure the check they belong is represented in the
> tags.d directory. Sometimes, that meant adding a blank file. (For all
> the automation, it should be noted that all tags that were listed in
> Test-For were being tested; Test-Against cannot be similarly
> validated.) Some tests used special features that will require more
> work. They were not converted.
>
> Going forward, there are many things you can do to make our test suite
> better. First of all, it is always a good idea to write more
> tests. While we already have many, one can rarely have enough. (In the
> future, we may group them somehow to make them easier to navigate.)
> For converted tests, please embrace the unit testing paradigm and help
> with the following:
>
> 1. Remove 'Exhaustive-Set: yes' from tags.d/options. (The default is
> no.)
>
> 2. Delete any tag files similar to tags.d/{$check}.tags that have only
> nuisance tags. Those are tags that appeared from unrelated checks you
> were not actually interested in testing. (At some point, lintian may
> also speed things up by skipping checks not present.) Please remove
> any file your test does not mean to validate. If two tag files appear
> useful separately, please consider duplicating the test, and place one
> file with each.
>
> 3. If you have the patience to add missing tags, please enable
> '--pedantic'.
>
> Now we have unit testing! Thank you!
Once again, thanks for working on improving lintian. As I mentioned, I
am happy to support the Abbreviated-Tags feature with only a minor
remark on the syntax (possibly retrofitted into the existing layout as
well).
For tags.d, I need some more examples to understand how the parts
interact.
Thanks,
~Niels
Reply to: