Hi Pranav, thanks to your work on the idba package. At first a note for your environmet setup. You obviously used dch to create the changelog entry. This added an invalid e-mail address as changelog owner. I recommend you set
[email protected] DEBFULLNAME='Pranav Ballaney' (or whatever e-mail address you prefer in your Debian work) in your environment to avoid this. Using an Ubuntu system for your work is fine but please note that dch adds a <Debrelease>ubuntu1 to the version instead of just bumping the <Debrelease> number. I can easily edit this before uploading but just giving you a hint about this here. On Sun, Feb 23, 2020 at 03:07:59PM +0530, Pranav Ballaney wrote: > Hi, > I have added autopkgtests for idba <https://salsa.debian.org/med-team/idba>. > (Closes #909716 <https://bugs.debian.org/909716>) > > I need a few suggestions on how these tests can be improved. > > 1. The test data can be generated using the sim_reads command that comes > with the package, but for some reason, it isn't installed when the package > is built. Ahhh, its always good if we can implement a 4 eyes principle to our packages. Just to introduce myself: I'm not a bioinformatician but a physicist who simply is able to create Debian packages and I'm not *using* these packages and barely know what they can be used for. I was packaging idba according to user request and I was simply installing those executables only that were requested by the user who asked me for. I'm now realising that idba build system builds a lot of other tools inside $(CURDIR)/bin which might (or might not?) be sensible to install. > I am being able to use it if I build the package from source on > my machine, but within the testing chroot, autopkgtest reports that this > command is not found. (UNTESTED!) Please have a look at commit 133e974096b40aedc7108258b286f74296894f3a where I've set PATH to make sure all binaries will be found. > If this command can be used while testing, the > downloads required for testing will be reduced from the current 25MB to a > mere 1.5MB, because the rest of the data can be generated as required. I have not verified the test suite but it is not possible to download anything while building the package since every package needs to be able to build on a non-networked machine. So every download has to be patched from the test suite anyway. > For > now, I have generated and added all the files manually, and included the > code used to generate this data in a README file inside > debian/tests/test_data, if it becomes possible to run these while testing > at a later stage. Its way more sensible to provide all needed tools inside the package. so please decide which of **all** the tools in $(CURDIR)/bin should end up in the binary package. You can install these easily by for instance adding a line like bin/sim_reads usr/bin to the file debian/install. > 2. To copy the test_data to a temp folder inside the testing chroot, I > have added an install file inside the debian folder. Is this the right way > to do it? In principle yes. However, if we really need to keep any large static data files (instead of generating these) I would prefere to add an additional Architecture: all package (for instance named idba-data). The rationale is that Architecture: any files are duplicated for every Debian architecture which is not bloating the Debian ftpmirror network with duplicated files but also creates a lot of network traffic. That's why we split up those binar independent files into a separate binary package. Moreover if the data are not used to actually run the package we would also bloat user machines who might just want to run idba in a cluster or so. In short: Please check prefere creation of the files in the test over static data. Having a few small files provided statically is fine, thought. Additional hint: Using the install file is fine, but its more elegant to use either the file debian/docs containing test_data or install the data in examples (either test_data subdir or directly by using debian/examples containing test_data/* (direct install) or test_data (in subdir) > I have also updated the changelog (and marked the bug resolved) and added a > README.test file inside debian. Very good! > Please let me know if I missed out anything, and if not, please review and > sponsor these changes. Provided that your time permits 1. Decide about what executables in $(CURDIR)/bin should be installed 2. Generate as much data as possible 3. Check whether test suite tries to download anything and ignore these tests or provide these data statically 4. If we need large static data files create an extra binary package Feel free to ask me about any of these steps if you might need any help! Hope this helps Andreas. -- http://fam-tille.de
