Re: Autopkgtest added for package staden

Étienne Mollier <[email protected]> Sun, 6 Jul 2025 15:24:13 +0200
Newsgroups	gmane.linux.debian.devel.medical
Message-ID	<[email protected]>
Hi Harish,

harish chavre, on 2025-07-05:
> I have added autopkgtest for the staden package [0]. The tests  try to include coverage for both the CLI and the graphical interface. Please take a moment to review and share any feedback or suggestions.
> [0]https://salsa.debian.org/med-team/staden/-/commit/d540fd92b916d478c0c6e7b75d1db97f823c3ada

Thanks for adding these checks, and nice reuse of the existing
logic in use by few other graphical applications!

I had a look at the test and have a couple of notes.  The
check_n_cleanup function is outputing some error messages, but
without failing the test:

	[…]
	check_n_cleanup
	/tmp/autopkgtest.3SbCQN/build.Ocz/src/debian/tests/run-unit-test: 31: ps: not found
	/tmp/autopkgtest.3SbCQN/build.Ocz/src/debian/tests/run-unit-test: 36: ps: not found
	/tmp/autopkgtest.3SbCQN/build.Ocz/src/debian/tests/run-unit-test: 41: ps: not found
	/tmp/autopkgtest.3SbCQN/wrapper.sh: Killing leaked background processes: 27 41 60 74 
	/tmp/autopkgtest.3SbCQN/wrapper.sh: 242: ps: not found
	autopkgtest [14:56:56]: test run-unit-test: -----------------------]
	autopkgtest [14:56:56]: test run-unit-test:  - - - - - - - - - - results - - - - - - - - - -
	run-unit-test        PASS

I'm concerned that the various occurrences of "||true" are going
to hide further genuine errors, and it would probably be
necessary to review the logic of the script.  You may want to
lookup the conditional block in edfbrowser's autopkgtest[1] for
an example; it is probably a more verbose construct than the
"&&" and "||" shorthands, but also less dangerous to handle.  As
of the "ps: not found" error, it can be resolved by adding a
missing dependency on procps in debian/tests/control.  I guess
this issue flown below the radar due to the test not failing.

[1]: https://salsa.debian.org/med-team/edfbrowser/-/blob/master/debian/tests/run-unit-test?ref_type=heads#L24-L30

The other issue I encountered after making some adjustments, is
that there might be processes not caught by the check_n_cleanup
step.  Manually running and killing a gap4 process preserves
nevertheless the window, when I run test steps by hand with a
real graphical interface.  The remaining process looks to be a
tclsh script gap.tcl which would need to be killed as well if it
is identified to still be running.  Remember, the point of the
check_n_cleanup function is to make sure that the graphical
interfaces of the software are still up after a short while.  If
the processes are not there anymore, then in all likelyhood,
they crashed earlier during the test, and thus the test should
be failed with a nonzero exit code.  If processes are still
there, then we can clean them up properly and move on to the end
of the test, successfully.

Please, make sure that procps is installed for each run by
declaring it in d/t/control dependencies, then that the test has
a chance to error when something abnormal happens per the logic
explained above, and finally that all processes associated to
the running programs have been identified and cleaned up
correctly at the end of the test.

Don't hesitate to reach out if you need some help.  I will be at
the DebCamp in the upcoming week and dedicate my time to Debian
activities.

Have a nice day,  :)
-- 
  .''`.  Étienne Mollier <[email protected]>
 : :' :  pgp: 8f91 b227 c7d6 f2b1 948c  8236 793c f67e 8f0d 11da
 `. `'   sent from /dev/pts/1, please excuse my verbosity
   `-    on air: Tiger Moth Tales - Story Tellers
signature.asc (application/pgp-signature, 833 B)