Hi Harish,
harish chavre, on 2025-04-03:
> I have successfully added autopkgtests for the bitseq
> package. I would appreciate it if you could review the changes
> and let me know if any modifications are needed.
Thanks for your work investigating how to prepare an autopkgtest
suite for the bitseq package, I have taken a close look at your
implementation and you will find my comments below. It's a bit
more involved than I anticipated when discussing yesterday, but
that would be a good opportunity to learn things. :)
* d/t/simple-commands-run: this script is a good start for
defining a set of superficial checks. It is always a good
idea to make sure that the basics options, such as --help,
are operating properly. At least it is better than nothing,
and it already allows catching a few classes of issues.
Note that such superficial checks are not eligible to
autopkgtest migration bonus however, and must be marked as
such in d/t/control with a restriction "superficial".
* d/t/test_bitseq.py: this file is not invoked by your
d/control file. My impression is that it was supposed to be
invoked via pytest, given the conventions followed for the
name of the script and remain in the source code of the
package, but I may have wrongly guessed. Can you confirm
the purpose of the file? If is not used by the package,
then it is better to not have it within the package, as
otherwise it creates clutter and makes the package source
code overall harder to understand.
* d/t/pytest: I was a bit confused by this file until it
occured to me that it might have been initially purposed to
trigger the tests in test_bitseq.py. I tend to find your
direct invocations via shell script easier to understand
than the idea of running commands via Python pytest script.
However I note that the script is a variation of superficial
checks that will have no chance to fail even in case of
problems, due to the following construction:
python3 "/usr/bin/$script" --help || echo "Warning: $script may not have a --help option"
The "|| echo …" condition will be met as soon as any failure
occurs in the script. Notably, a script that used to work
with --help and suddently crashes will still pass by issuing
the warning. If the script is lacking --help support, then
it is better to not attempt to run it and find another way
to make sure it is nevertheless usable. By the way, please
could you find a better name for that script? Even if it
runs pytest, it is not a pytest implementation and creates
confusion when trying to understand how the autopkgtests are
wrapped. I would probably go as far as merging the content
of the script d/t/pytest into d/t/simple-commands-run as
they seem to share similar purpose, but you may have other
ideas in mind.
* d/t/control: the needs-root restriction will enforce the run
of autopkgtest via the root account. This may be necessary
for testing servers on well known ports and other corner
cases, but in general, and especially with scientific and
medical applications, this restriction should not be needed.
Please remove the needs-root restriction and make sure that
the test suite is written in such a way that it can run
without needing administrative permissions.
If that can help you wrap up more involved autopkgtests for
bitseq, I have found by reading the project's README.md file
that upstream have a wiki page[1] in which they document how to
use bitseq. Getting some test data along may be the trickiest
part, because you would need to ensure that data you want to use
is DFSG compliant, or be reasonably lightweight to avoid
cluttering the package. I usually resolve that problem by
repurposing test or example data from other existing packages
when I work on a package lacking such data. Also, the Debian
Med team accumulated already many autopkgtest over time, so
don't hesitate to look them up for inspiration[2].
[1]: https://github.com/BitSeq/BitSeq/wiki/Basic-usage
[2]: https://salsa.debian.org/med-team/abpoa/-/blob/master/debian/tests/run-unit-test?ref_type=heads
As a side note, you probably want to setup your .gitconfig so
that your Author field in your git commits matches your actual
contact address. Currently it shows like this, which is not
helpful contact address to trigger discussions when needed:
Author: root <root@HARISH.harish.dev>
You would probably prefer reading your real email address here.
And that is all for now. I hope I was not too overwhelming with
my review. Again, Thanks for your work in attempting to improve
the quality of the bitseq package!
Have a nice day, :)
--
.''`. Étienne Mollier <emollier@debian.org>
: :' : pgp: 8f91 b227 c7d6 f2b1 948c 8236 793c f67e 8f0d 11da
`. `' sent from /dev/pts/3, please excuse my verbosity
`- on air: Moon Safari - Lover's End Pt. III: Skellefteå…
Attachment:
signature.asc
Description: PGP signature