home bbs files messages ]

Forums before death by AOL, social media and spammers... "We can't have nice things"

   linux.debian.maint.emacsen      Maintaining Emacs on Debian      675 messages   

[   << oldest   |   < older   |   list   |   newer >   |   newest >>   ]

   Message 620 of 675   
   Xiyue Deng to Xiyue Deng   
   Bug#1125765: dh-make-elpa: new Perl-base   
   28 Jan 26 01:40:01   
   
   XPost: linux.debian.bugs.dist   
   From: manphiz@gmail.com   
      
   Control: tags -1 dh-make-elpa: new Perl-based integration tests   
      
   Xiyue Deng  writes:   
      
   > 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   
      
   [continued in next message]   
      
   --- SoupGate-Win32 v1.05   
    * Origin: you cannot sedate... all the things you hate (1:229/2)   

[   << oldest   |   < older   |   list   |   newer >   |   newest >>   ]


(c) 1994,  bbs@darkrealms.ca