Re: esh-proc test failures

[Jim Porter <jporterbugs@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1631 bytes --]

On 8/30/2022 9:51 AM, Jim Porter wrote:
> On 8/29/2022 8:18 PM, Jim Porter wrote:
>> Merged as f9250c5ebc1730bf3bed4382549433f52f7ef9ca. Hopefully this 
>> fixes EMBA; if there are any further issues anyone encounters, just 
>> let me know.
> 
> Apparently it doesn't. I've confirmed again locally that my patch does 
> fix *an* issue with 'eshell-pipe-broken', but I guess it's not the same 
> issue as what's on EMBA. I'll see if I can fix EMBA for real, or failing 
> that, we can disable the tests again.

Ok, it looks like this is now failing only for native-comp builds. I 
think the patch I pushed *does* fix the bug, but the regression tests 
need a bit of extra help for native-comp builds.

The attached diff should make it easier for others to see what's going 
on: with "slow-echo", Eshell will always print output after the "sh" 
subprocess has finished, triggering the bug. Then, we need to set 
'debug-on-error' to nil in order to fix this for real. That's because 
Eshell uses 'condition-case-unless-debug' inside 
'eshell-condition-case'. If anyone would like to test this out locally, 
just try out the diff with native-comp on or off and with/without the 
'debug-on-error' overrides.

However, I'm a bit concerned that there's another, more-general bug 
here. Why would native-comp and non-native-comp differ here? I think 
native-comp is doing the right thing, but maybe not... does anyone have 
any ideas here?

In any case, assuming there are no objections, I'll merge a patch that 
just sets 'debug-on-error' to nil in the appropriate spots in 
esh-proc-tests.el in the next day or so. That should fix EMBA.

[-- Attachment #2: fix-tests-under-native-comp.diff --]
[-- Type: text/plain, Size: 2195 bytes --]

diff --git a/test/lisp/eshell/esh-proc-tests.el b/test/lisp/eshell/esh-proc-tests.el
index 2369bb5cc0..51c8f7fcb8 100644
--- a/test/lisp/eshell/esh-proc-tests.el
+++ b/test/lisp/eshell/esh-proc-tests.el
@@ -22,6 +22,7 @@
 (require 'ert)
 (require 'esh-mode)
 (require 'eshell)
+(require 'em-basic)                     ; FIXME: remove this
 
 (require 'eshell-tests-helpers
          (expand-file-name "eshell-tests-helpers"
@@ -35,6 +36,15 @@ esh-proc-test--detect-pty-cmd
           "if [ -t 2 ]; then echo stderr; fi"
           "'"))
 
+;; FIXME: remove this before committing vvv
+(defun eshell/slow-echo (&rest args)
+  "A version of `eshell/echo' that takes a while to print."
+  (let ((total 0))                      ; Busy-loop for a while.
+    (dotimes (i 1000000)
+      (setq total (+ total i))))
+  (eshell/echo args))
+;; FIXME: remove this before committing ^^^
+
 ;;; Tests:
 
 (ert-deftest esh-proc-test/sigpipe-exits-process ()
@@ -76,17 +86,23 @@ esh-proc-test/pipeline-connection-type/middle
 pipeline."
   (skip-unless (and (executable-find "sh")
                     (executable-find "cat")))
-  (eshell-command-result-equal
-   (concat "echo | " esh-proc-test--detect-pty-cmd " | cat")
-   nil))
+  ;; An `eshell-pipe-broken' signal might occur internally; let Eshell
+  ;; handle it!
+  (let ((debug-on-error nil))
+    (eshell-command-result-equal
+     (concat "slow-echo hi | " esh-proc-test--detect-pty-cmd " | cat")
+     nil)))
 
 (ert-deftest esh-proc-test/pipeline-connection-type/last ()
   "Test that only output streams are PTYs when a command ends a pipeline."
   (skip-unless (executable-find "sh"))
-  (eshell-command-result-equal
-   (concat "echo | " esh-proc-test--detect-pty-cmd)
-   (unless (eq system-type 'windows-nt)
-     "stdout\nstderr\n")))
+  ;; An `eshell-pipe-broken' signal might occur internally; let Eshell
+  ;; handle it!
+  (let ((debug-on-error nil))
+    (eshell-command-result-equal
+     (concat "slow-echo hi | " esh-proc-test--detect-pty-cmd)
+     (unless (eq system-type 'windows-nt)
+       "stdout\nstderr\n"))))
 
 (ert-deftest esh-proc-test/kill-pipeline ()
   "Test that killing a pipeline of processes only emits a single