Subject: Re: bug#50630: [PATCH] Add tests for insert-directory

From: Eli Zaretskii <[email-masked]>
Date: Sat, 25 Sep 2021 17:55:16 +0300

> Date: Sat, 25 Sep 2021 14:29:17 +0000
> From: John Cummings <[email-masked]>
> Cc: [email-masked]
> 
> > 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.

At the time, I looked at what happened on Windows, and didn't see the
problem, because ls-lisp does

    (get-free-disk-space ".")

and the default-directory of the buffer at that point is already set
to the directory whose listing we will present.

>  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

That recipe invokes insert-directory in some arbitrary way, which is
probably different from what happens when one invokes Dired.  So I'm
not sure we should worry about that.  But anyway, I tried that recipe
on MS-Windows, and I don't see the problem: the values shown on the
first line reflect my home directory, not pwd-a or pwd-b.

>  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.

I suggest that you remove the special treatment of ls-lisp from that
test.  As I say above, I don't expect it to need any special
treatment, I expect it to pass the test without any changes.  And if
it fails, let's take it from there.

In any case, I see no reason to move the ls-lisp specific test of
insert-directory to ls-lisp-tests.el: since we are testing
insert-directory, the natural place for that test is in
files-tests.el, regardless of what platform-specific tricks are at
work behind the scenes.

> I want to avoid situations where someone runs files-tests.el and it
> actually tests the insert-directory implementation from ls-lisp.el.

I see no reason to avoid that.  The entry point is insert-directory,
and that is what's being tested here.

Thanks.