Re: [PATCH] Async evaluation in ob-shell

[Matt <matt@excalamus.com>]

 ---- On Sat, 11 Feb 2023 06:44:56 -0500  Ihor Radchenko  wrote --- 
 > 1. You should provide all the docstrings.
 > 2. I generally feel that separate async and separate session code are
 >    either duplicating the same code or edge cases considered by session
 >    code may popup in async code unexpectedly.

Excellent points.  Thank you.

As part of the commit, I want to include tests.

How to test an async block is non-obvious.  The initial evaluation returns a uuid which returns immediately and can be checked using a regex:

(defconst test-ob-shell/uuid-regex
  "[0-9a-fA-F]\\{8\\}\\b-[0-9a-fA-F]\\{4\\}\\b-[0-9a-fA-F]\\{4\\}\\b-[0-9a-fA-F]\\{4\\}\\b-[0-9a-fA-F]\\{12\\}")

Technically, this is a ob-comint aspect and may be more rightly included in (the currently non-existant) tests for that module.

Checking the final result from the callback is trickier.   The following works, but requires advice (which could potentially persist beyond the test) and a delay (which slows testing overall and whose duration likely depends on the hardware the test runs on).  Without the delay, I could not get the callback to execute within the test.  It would execute when running manually in an Org buffer, however.  I'm not sure why. 

(ert-deftest test-ob-shell/session-async-evaluation ()
  (let ((session-name "test-ob-shell/session-async-evaluation")
        (kill-buffer-query-functions nil)
        result)
    ;; perform check after the callback executes which looks for the
    ;; expected result
    (advice-add
     'ob-shell-async-chunk-callback
     :filter-return
     (lambda (&rest r)
       (let ((result (car r)))
         (if (not (string= result "1\n2\n"))
             (ert-fail (format "Expected 1\n2\n: %s" result)))
         result))
     `((name . ,session-name)))
    ;; always remove the advice, regardless of test outcome
    (unwind-protect
        (org-test-with-temp-text
            (concat "#+begin_src sh :session " session-name " :async t
echo 1
echo 2<point>
#+end_src")
          ;; execute the block; delay momentarily so that the callback
          ;; executes
          (setq result (org-trim (org-babel-execute-src-block)))
          (if (should
               (and
                ;; if the block runs...
                (string-match
                 test-ob-shell/uuid-regex
                 result)
                ;; ...and the callback executes without fail
                (not (sleep-for 0.1))))
              ;; clean up buffer on success
              (kill-buffer session-name)))
      (advice-remove 'ob-shell-async-chunk-callback session-name))))

This works for me using the last patch.

Thoughts?