XPost: linux.debian.bugs.dist   
   From: manphiz@gmail.com   
      
   Hi Richard,   
      
   Thanks very much for the review and suggestions! Please see my replies   
   below inline.   
      
   Richard Lewis writes:   
      
   > On Sat, 17 Jan 2026 at 04:25, Xiyue Deng wrote:   
   >>   
   >>   
   >> I have been working on a new Perl-based unit tests and I think it's now   
   >> in a usable form. It runs dh-make-elpa in a temp directory and compares   
   >> the contents of its output with the expected ones.   
   >   
   >> [1] https://salsa.debian.org/emacsen-team/dh-make-elpa/-/merge_requests/6   
   > (i cant review the perl parts in detail though)   
   >   
   > I've used this approach in a couple of other packages, so thought i'd   
   > offer some thoughts based on my experiences   
   >   
   > The pros include   
   > - the test is what the underlying code is actually producing, so you   
   > get confidence that the test is actually testing what a user sees   
   > - you can add tests without needing to understand the code at all, and   
   > the underlying code doesnt need to have any support for "testing"   
   > - it's esp good when you want to know if the output has changed -- i   
   > think this is a good fit for something like dh-make-elpa where   
   > if the output changes it may be unintentional, and checking "by   
   > hand" is tedious   
   >   
      
   You helped cover several of the design goals of those tests. Thanks!   
      
   > The cons include   
   > - someone has to keep the test data up-to-date   
      
   I have written t/utils/check_diff_in_test_cases for this exact reason,   
   which can help update the contents in `expected/' with `--update'.   
   Manually doing that was really a painful experience. I would expect one   
   to have carefully inspected the diffs before committing.   
      
   > - i think a lot of people in debian commit things before bothering to   
   > test - more git commits fixing test failures   
   > - rebasing branches and MRs that change output can be a HUGE pain   
      
   This is what the tests are for IMHO. Now we have Salsa CI and   
   autopkgtest to detect the test failures, and who breaks them should fix   
   them. I think it's better than breaking the program unintentionally.   
      
   > - most test failures will be "the test suite is out of date" rather   
   > than "there is a bug in the package", this can de-sensitize you from   
   > investigating, as once   
   > the "expected" is updated, any issues are hidden.   
      
   Yes. One needs to carefully inspect that the updated `expected/'   
   contents are intended before committing.   
      
   This would also promote smaller change per commit, like changing just   
   one aspect and update the tests accordingly in one commit. Changing too   
   many places at once makes it hard to understand and even harder to   
   reason about the diffs and hence should be discouraged.   
      
   > - you are exposed to changes in other packages (*)see below   
   > - (this may not apply for this package?) you are exposed to   
   > differences in the build environment - it may pass in a local sbuild   
   > but fail on salsa, or in the debian build system, or debusine, or in   
   > stable vs unstable   
      
   IMHO this should expose a bug either in the package or the tests and   
   should be fixed. So far it passes local sbuild and Salsa CI at least :P   
      
   > - generally anything that makes the output non-reproducible, like   
   > timestamps are a pain.   
      
   Indeed. Which is why I generate the diff by excluding any lines that   
   includes a timestamp[1] (I'm lazy :P). Admittedly this is not very   
   robust.   
      
   I have seen some other software sanitize any timestamp to use "X" to   
   replace each character, which may be a better way to handle this.   
      
   > - everyone making changes to the package will need to understand how it works   
      
   Hopefully it's not too complicated to understand: it's basically running   
   `diff' for comparison :)   
      
   > - in every case ive tried, yu end up needing a way to "filter" the   
   > 'actual' output before comparison to avoid all the unreproducible   
   > bits!   
   >   
      
   Ack. Similar as the point on timestamp above.   
      
   > (i think the benefits are more than the costs for a "stable",   
   > slow-moving package with an active maintainer!)   
   >   
      
   Thanks!   
      
   > *i did notice that you are hard-coding the contents of a "new git   
   > repos" including all the hook templates (eg   
   > t/data/github_team/repo/git/hooks/commit-msg.sample )- this means   
   > when a new version of git makes a change (even changing the comments   
   > in a template, the test will fail (and maybe block a new git from   
   > migrating?). I dont know if you can instead do a fresh "git init" and   
   > copy the .git into the "expected" directory before doing the   
   > comparison, rather than holding it in the repo?   
      
   The part of `.git' that matters is the `.git/config' file, and   
   dh-make-elpa uses the `upstream' branch info for generating the   
   `debian/watch' file, so the contents of hooks and info directories don't   
   really matter. As a lazy person, I just did `git init' and edit   
   `.git/config' for the tests. And it turns out those are not needed at   
      
   [continued in next message]   
      
   --- SoupGate-Win32 v1.05   
    * Origin: you cannot sedate... all the things you hate (1:229/2)   
|