Re: Shell quoting in Eshell (was: eshell-batch-file)

[Jim Porter]

On 6/15/2024 2:38 AM, Eli Zaretskii wrote:
> I get this failure:
[snip]
>    Test esh-proc-test/kill-pipeline condition:
>        (ert-test-failed
>         ((should
>          (string-match-p (rx ...)
>                          (buffer-substring-no-properties output-start ...)))
>         :form
>         (string-match-p "\\(?:\\(?:interrupt\\|killed\\(?:: 9\\)?\\)\n\\)"
>                         "")
>         :value nil))
>       FAILED   4/23  esh-proc-test/kill-pipeline (0.131174 sec) at 
> lisp/eshell/esh-proc-tests.el:283
>
> Why do we expect to see "killed" or "interrupt" in this case?

Eshell intends to print the exit message from the process sentinel of 
the tail process here (unless the message starts with "finished" or 
"exited"). After thinking this over, I realized that the test failure 
you encountered was actually a race condition: Eshell tries to kill all 
the processes in the pipeline, which -- if Eshell is fast enough -- 
sends a signal to the tail process, making us print the exit message. 
However, if Eshell is slow, the tail process might exit on its own 
before we can signal it, leading to no message.

I've now fixed this test so that we check that there are zero or one 
exit messages (the old code was too lenient in some regards).

I think the only remaining thing to do here is to figure out why you 
needed to change 'eshell-command-result--equal'. I imagine this is 
because I made a mistake somewhere with case normalization (my guess is 
in the globbing tests). Hopefully it's just a problem with the tests, 
but it could be that there's a bug hiding in Eshell proper.

Could you try the diff below and report back the failures when you get a 
chance?


--------------------------------------------------


diff --git a/test/lisp/eshell/eshell-tests-helpers.el 
b/test/lisp/eshell/eshell-tests-helpers.el
index acbe57a7283..bfd829c95e9 100644
--- a/test/lisp/eshell/eshell-tests-helpers.el
+++ b/test/lisp/eshell/eshell-tests-helpers.el
@@ -179,12 +179,7 @@ eshell-test-command-result

 (defun eshell-command-result--equal (_command actual expected)
   "Compare the ACTUAL result of a COMMAND with its EXPECTED value."
-  (or (equal actual expected)
-      ;; Compare case-isensitively on case-insensitive filesystems.
-      (and (memq system-type '(windows-nt ms-dos))
-           (stringp actual)
-           (stringp expected)
-           (string-equal-ignore-case actual expected))))
+  (equal actual expected))

 (defun eshell-command-result--equal-explainer (command actual expected)
   "Explain the result of `eshell-command-result--equal'."