From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: John Cummings Newsgroups: gmane.emacs.bugs Subject: bug#50630: [PATCH] Add tests for insert-directory Date: Sat, 25 Sep 2021 14:29:17 +0000 Message-ID: References: <83a6k1rxrc.fsf@gnu.org> <83ilyorei3.fsf@gnu.org> Reply-To: John Cummings Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="23420"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 50630@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Sep 25 16:32:48 2021 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mU8jP-0005wn-FV for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 25 Sep 2021 16:32:47 +0200 Original-Received: from localhost ([::1]:42470 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mU8jO-00010g-6Z for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 25 Sep 2021 10:32:46 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:39626) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mU8gl-00073n-Am for bug-gnu-emacs@gnu.org; Sat, 25 Sep 2021 10:30:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:50968) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mU8gk-0003q8-QS for bug-gnu-emacs@gnu.org; Sat, 25 Sep 2021 10:30:03 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mU8gk-00005Y-N8 for bug-gnu-emacs@gnu.org; Sat, 25 Sep 2021 10:30:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: John Cummings Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 25 Sep 2021 14:30:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 50630 X-GNU-PR-Package: emacs Original-Received: via spool by 50630-submit@debbugs.gnu.org id=B50630.163258017432739 (code B ref 50630); Sat, 25 Sep 2021 14:30:02 +0000 Original-Received: (at 50630) by debbugs.gnu.org; 25 Sep 2021 14:29:34 +0000 Original-Received: from localhost ([127.0.0.1]:34281 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mU8gI-0008Vz-8X for submit@debbugs.gnu.org; Sat, 25 Sep 2021 10:29:34 -0400 Original-Received: from mail-40131.protonmail.ch ([185.70.40.131]:13726) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mU8gG-0008Vk-7q for 50630@debbugs.gnu.org; Sat, 25 Sep 2021 10:29:32 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rootabega.net; s=protonmail; t=1632580161; bh=biTO/NSpoTpghCdEd4MFBT0pYo1gDO6ACExhyNaBlnI=; h=Date:To:From:Cc:Reply-To:Subject:In-Reply-To:References:From; b=h1WdNxMYGItctH26+aYm1c8YNhKTsgVJXniPH+WbVIB+Z+rHavJRwaCxnmwWxmhXX gboOz6ByCWoDnIsaYFeJHMrEZM+GGmxHgnilgiclfzZ9emzCAKYssw0mOEfVigSHMC aZq7kNcwW1k0WWyIa5ZBBWUlcLoTQrW551CPPatV1Zpg0Tp+XK0g+ej0aSl6PFEUkN gxlZqQcElB1YhTlXFtp+Rk57rWET6s5AfuYRvOvd4HpcLQAw2JjikfQ+eborxK9XGr uEZP//1KNjRIWARtapLWqghWFY5j5T5xErOt/SrBfFuzed011XjvdWrFXqJm5Sm5IM kcmlJeJDtqn3Q== In-Reply-To: <83ilyorei3.fsf@gnu.org> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:215446 Archived-At: Eli Zaretskii wrote: >> Date: Sat, 25 Sep 2021 11:38:08 +0000 >> From: John Cummings >> Cc: 50630@debbugs.gnu.org >> >> Eli Zaretskii wrote: >> >> >> Date: Fri, 24 Sep 2021 19:58:25 +0000 >> >> From: John Cummings >> >> >> >> Along those lines, I also attempted to skip the test when ls-lisp >> >> would be used during files-tests.el, which I predict might happen whe= n >> >> building on Windows? >> > >> > Why did you need to skip those tests? Can you elaborate? >> >> ls-lisp has its own implementation of insert-directory, which >> duplicates the bug, and which will also duplicate the fix when I >> submit it soon. > > Some context is probably lost here: which bug are you talking about? 50630. A breakdown of the context from other messages in this thread that've driven my responses so far: I found that list-directory in files.el reported the free space of default-directory instead of the function's DIRNAME argument. The fix I submitted addressed that in insert-directory in files.el, so it seems like people are OK with treating this as a problem in insert-directory. i.e. the same incorrect free space was reported whether you called list-directory or insert-directory. I and others found that the same problem was duplicated in ls-lisp.el and tramp-sh.el, and I provided instructions to reproduce attached earlier in the thread: https://lists.gnu.org/archive/html/bug-gnu-emacs/2021-09/msg01813.html You can confirm the bug in tramp-sh.el and ls-lisp.el against master (currently c17eded545) , and you can confirm the bug in files.el against revision a462ccd536 and earlier. I've now submitted a regression test for 50630, which I thought would be appropriate to include in the 50630 context. I also asked elsewhere in the thread whether we shouldn't also handle the instances of the bug in ls-lisp.el and tramp-sh.el in that same bug instead of creating additional bugs for what is essentially the same small bug duplicated a few times. But it's just a question, and if the leaders want to have one bug for every library, I'll understand. > And if that bug will be fixed in ls-lisp, then why do you need to > single ls-lisp out? I want to avoid situations where someone runs files-tests.el and it actually tests the insert-directory implementation from ls-lisp.el. My plan is to try to reuse, or at worst, copy the testing helpers that I already submitted for files.el to perform the same test scenario in ls-lisp-tests.el and in an appropriate place in the tramp tests. But as I mentioned in another reply, I also understand that making files-tests.el have to worry about this might not be appropriate, and if there's another way to handle that, I'd be glad to hear about it and will be researching that myself, too. >> I admit I may not correctly or completely understand the way that >> ls-lisp could relate to a batch ert run at build time, or how that >> varies between architectures. > > Whenever you invoke insert-directory, on MS-Windows it will invoke > ls-lisp. That aligns with my reasoning, and so someone running the 50630 regression test for files.el on MS-Windows would actually be testing ls-lisp.el, and would see inaccurate regression status for files.el. As of today, the regression test (files-tests-bug-50630) would fail, and I wouldn't want to have me adding this test be a reason for any failing MS-Windows builds or other control gates. In the future, anyone running that test on MS-Windows wouldn't see a failure, but they still wouldn't have tested the code that files-tests.el is intended to test. >> I personally find the variable a little confusing, >> and think that negating its name and meaning would be more natural, >> but the documentation does make it clear, even if I have to think >> about it for a bit every time I set it. > > You are talking about ls-lisp-use-insert-directory-program? Why it is > confusing? ls-lisp by default doesn't use any programs, it accesses > the files' attributes using Emacs Lisp APIs; so the "program" part is > a reference to the fact that on Posix platforms insert-directory uses > the 'ls' program instead. I only mentioned that because I thought it might have been a reason why it was hard to understand what I was doing with that variable in files-tests.el. The reason I find it confusing, which could be personal to me, is that I think it's more natural that a library's config variables would need to be non-nil when wanting to enable a feature or function of that library. So a name like 'ls-lisp-use-ls-lisp-insert-directory', and inverting the values, would have been easier for me to remember, and simpler to express: (and (featurep 'ls-lisp) ls-lisp-use-ls-lisp-insert-directory) would mean that we're using the ls-lisp implementation.