Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: Lockywolf <for_emacs_1@lockywolf.net>
>> From: Lockywolf <for_emacs_1@lockywolf.net>
>> Date: Thu, 07 Aug 2025 15:01:59 +0800
>> 
>> Updated version of the patch attached:
>
> Thanks, comments below.
>
>> >From a5a489689f5361c4a86749ac3c9b33e83b23e528 Mon Sep 17 00:00:00 2001
>> From: Lockywolf <for_emacs_1@lockywolf.net>
>> Date: Thu, 24 Jul 2025 21:07:10 +0800
>> Subject: [PATCH] ispell.el: Add 39 tests.
>> 
>> * test/lisp/textmodes/ispell-tests/*: Add 36 tests for basic utils
>>   used in ispell.el.
>>    ispell-program-name
>>    ispell-with-safe-default-directory
>>    ispell-call-process
>>    ispell-create-debug-buffer
>>    ispell-valid-dictionary-list
>>    ispell-add-per-file-word-list
>>    ispell-buffer-local-words
>>    ispell-init-process
>>    ispell-buffer-local-dict
>> * test/lisp/textmodes/ispell-resources/fake-aspell: Add a mock
>>   `aspell' for use in ispell.el test.
>
> Please mention the bug number in the commit log message.
>
>> diff --git a/test/lisp/textmodes/ispell-resources/fake-aspell.bash b/test/lisp/textmodes/ispell-resources/fake-aspell.bash
>> new file mode 100755
>> index 00000000000..4406a18a22e
>> --- /dev/null
>> +++ b/test/lisp/textmodes/ispell-resources/fake-aspell.bash
>> @@ -0,0 +1,2 @@
>> +#!/bin/bash
>> +printf  '%s\n' "@(#) International Ispell Version 3.1.20 (but really Aspell 0.59.800)"
>
> I asked to use Emacs instead of a Bash script?  If that causes
> problems, let's discuss that, okay?

Aspell needs to parse the "-vv" command-line argument, and Emacs
launched in batch mode cannot do that.  Or I did not manage to make it.

I am not intentionally ignoring your suggestions, some of my responses
got lost while moving from emacs-devel to the bug tracker.

>
>> +   (with-temp-buffer
>> +     (let ((default-directory "86e44985-cfba-43ba-98dc-73be46addbc2"))
>> +       (ispell-call-process "emacs" nil t nil '("--batch" "-q" "-nw" "--eval" "(progn (message default-directory) (kill-emacs))"))
>
> There's no need for "-nw", "--batch" implies it.  Also, I suggest to
> use "-Q" instead of "-q", so that site-init file is not read.
>
> Same comment in the other places you invoke Emacs as a subordinate
> process.

Fixed.

>> +          (setopt ispell-library-directory "/tmp")
>
> Please use temporary-file-directory, not a literal "/tmp" (which is
> not portable), here and elsewhere.

Fixed.

>> +    (let ((test-dict "nonexistent")
>> +          (test-pdict "/tmp/lnonexistent.txt"))
>                          ^^^^^^^^^^^^^^^^^^^^^^^
> Likewise here: please use
>
>    (expand-file-name "lnonexistent.txt" temporary-file-directory)
>
> instead.  (There are several other places in the patch with the same
> problem.)

This is not a file path.  This is just a string which looks like a file
path.  If something tries to access this as if it is a path during the
execution of the test, this is an error.