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

[John Cummings <john@rootabega.net>]

Eli Zaretskii <eliz@gnu.org> wrote:

>> Date: Fri, 24 Sep 2021 19:58:25 +0000
>> From: John Cummings <john@rootabega.net>
>>
>> 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 when
>> 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.  (Let me know if the recipe to confirm the bug is
present in ls-lisp.el attached earlier doesn't work for you.)  So if
ls-lisp was "active" during this test, it could be a false
positive/negative for this test, depending on whether ls-lisp had been
fixed yet. (See my reply to your later question in this message for
what I mean when I say ls-lisp is "active", because just requiring the
library isn't enough.)

On my GNU/Linux system, ls-lisp is not active when running this test
from the Makefile, but I don't want to assume that that is true of
every system. Since Windows GNU Emacs uses ls-lisp.el by default when
running, I thought it might be one of those exceptions at build time
as well. I confirmed that it's also true if someone runs this test
interactively after having activated ls-lisp, regardless of the
system. That is, I activated ls-lisp, eval'd files-tests.el, and ran
ert for that file, and the test failed because it used the buggy
insert-directory from ls-lisp. So even if building a Windows Emacs (or
any system) does not use ls-lisp by default, is there any harm in
having this skip condition (other than potentially confusing the
reader)?

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. If it's NOT something to worry about
during a 'make', would I still need to worry about someone who ran
this test after enabling ls-lisp manually? Perhaps I could fail the
test so the user knows what they did didn't make sense?

Also, note that this is only true for the 50630 regression test. That
may have been the wrong choice. Perhaps it would make sense to run any
test of insert-directory's implementation if and only if ls-lisp.el is
not active?

>> +       ;;Try to verify that insert-directory will come from files.el,
>> +       ;;not ls-lisp.el. Windows builds will probably use ls-lisp.el
>> +       ;;by default, which will invalidate some tests.
>> +       (insert-directory-program-used (or (not (featurep 'ls-lisp))
>> +                                          ls-lisp-use-insert-directory-program)))

> I guess this is part of the same question I asked above?  Because I
> don't think I understand what you are trying to do here and why.

Yes, my logic was that this being non-nil means the test will be using
the Emacs-default insert-directory implementation from files.el, and
not the ls-lisp.el one.  In order to use the ls-lisp one (what I've
referred to as ls-lisp being "active" previously), the ls-lisp library
needs to be loaded, and the variable on the second line needs to be
nil, otherwise ls-lisp will just delegate back to files.el for
insert-directory. 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.