[BUG] FAILED test-ob-python/session-multiline

[BUG] FAILED test-ob-python/session-multiline

[Ihor Radchenko]

Hi,

We have fairly regular CI test failures for one of the ob-python tests.
The test does not fail _every_ time, but I keep seeing the problem in
various Emacs versions, including Emacs 29.

Example log: https://builds.sr.ht/~bzg/job/1047678#task-build

In the test the result somehow includes prompt:

Test test-ob-python/session-multiline condition:
    (ert-test-failed
     ((should
       (equal "20"
	      (org-test-with-temp-text "#+begin_src python :session :results output\n  foo = 0\n  for _ in range(10):\n      foo += 1\n\n      foo += 1\n\n  print(foo)\n#+end_src" ...)))
      :form
      (equal "20" ">>> 20")
      :value nil :explanation


-->   (arrays-of-different-length 2 6 "20" ">>> 20" first-mismatch-at 0)))


   FAILED   376/1256  test-ob-python/session-multiline (0.011955 sec) at ../lisp/test-ob-python.el:105

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [BUG] FAILED test-ob-python/session-multiline

[Jack Kamm]

Ihor Radchenko <yantar92@posteo.net> writes:

> Hi,
>
> We have fairly regular CI test failures for one of the ob-python tests.
> The test does not fail _every_ time, but I keep seeing the problem in
> various Emacs versions, including Emacs 29.
>
> Example log: https://builds.sr.ht/~bzg/job/1047678#task-build
>
> In the test the result somehow includes prompt:
>
> Test test-ob-python/session-multiline condition:
>     (ert-test-failed
>      ((should
>        (equal "20"
> 	      (org-test-with-temp-text "#+begin_src python :session :results output\n  foo = 0\n  for _ in range(10):\n      foo += 1\n\n      foo += 1\n\n  print(foo)\n#+end_src" ...)))
>       :form
>       (equal "20" ">>> 20")
>       :value nil :explanation
>
>
> -->   (arrays-of-different-length 2 6 "20" ">>> 20" first-mismatch-at 0)))
>
>
>    FAILED   376/1256  test-ob-python/session-multiline (0.011955 sec) at ../lisp/test-ob-python.el:105

Hmmm. Do you have an idea of how long this has been happening, and how
frequently it breaks?

My first suspicion is the large ob-python commit I pushed on Tuesday:

https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=579e8c572345c42ad581d3ddf0f484567d55a787

But that commit shouldn't affect ":results output", at least not
intentionally.

Also, do you have any tips for searching or navigating the failing CI
builds?  I tried going to https://builds.sr.ht/~bzg, but it didn't
include the failed build you linked to (which I guess is an "unlisted"
build).

Re: [BUG] FAILED test-ob-python/session-multiline

[Ihor Radchenko]

Jack Kamm <jackkamm@gmail.com> writes:

>>    FAILED   376/1256  test-ob-python/session-multiline (0.011955 sec) at ../lisp/test-ob-python.el:105
>
> Hmmm. Do you have an idea of how long this has been happening, and how
> frequently it breaks?

For months.

> My first suspicion is the large ob-python commit I pushed on Tuesday:
>
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=579e8c572345c42ad581d3ddf0f484567d55a787

So, should not be the recent commit.

> Also, do you have any tips for searching or navigating the failing CI
> builds?  I tried going to https://builds.sr.ht/~bzg, but it didn't
> include the failed build you linked to (which I guess is an "unlisted"
> build).

https://lists.sr.ht/~bzg/org-build-failures

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [BUG] FAILED test-ob-python/session-multiline

[Jack Kamm]

Ihor Radchenko <yantar92@posteo.net> writes:

> Jack Kamm <jackkamm@gmail.com> writes:
>
>>>    FAILED   376/1256  test-ob-python/session-multiline (0.011955 sec) at ../lisp/test-ob-python.el:105
>>
>> Hmmm. Do you have an idea of how long this has been happening, and how
>> frequently it breaks?
>
> For months.
>
>> My first suspicion is the large ob-python commit I pushed on Tuesday:
>>
>> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=579e8c572345c42ad581d3ddf0f484567d55a787
>
> So, should not be the recent commit.

This one might take some time to fix, since it's hard to reproduce and
I'm not sure the cause of it. But here are 3 different solutions I am
considering now:

1. In addition to printing `org-babel-python-eoe-indicator' after
   execution, we could also print out a "beginning of execution"
   indicator before execution, and then capture the output between the
   beginning and end indicators. This is how the async session
   execution works, and should avoid any possibility of capturing
   prompts.

2. Instead of relying on our own custom `org-babel-python-send-string',
   we could try switching to python.el's
   `python-shell-send-string-no-output', which is probably more
   robust. This would also allow removing the ugly
   `org-babel-python-eoe-indicator' we currently print.

   Downside is that the output would not be echoed into the session
   anymore. To fix that, we could manually insert the captured output
   into the comint session buffer after execution.  Alternatively, we
   could add an argument to `python-shell-send-string-no-output' to
   avoid suppressing output, submit it upstream to python.el, and then
   backport to Org to support older emacs versions.

3. Revisit a series of commits I made in 2020, which was supposed to
   make session evaluation more robust, and was inspired by
   `python-shell-send-string-no-output':
   https://git.sr.ht/~bzg/org-mode/commit/4df12ea39

   However, I had to partially revert that work, due to compatibility
   issue with emacs 26.3:
   https://list.orgmode.org/871rjcan53.fsf@kyleam.com/

   I think we no longer support emacs 26.3, so I could potentially
   revisit this now -- but it's been a few years and will take some
   time to refresh my memory about this.

Of these options, Option 1 is the easiest, and the most certain to solve
this bug. Options 2 and 3 are more difficult and riskier, but would have
other benefits if they work: we can remove the ugly
`org-babel-python-eoe-indicator' that is currently printed to the
session, and/or reduce long-term maintenance burden by relying on
python.el's implementation for capturing output.

Re: [BUG] FAILED test-ob-python/session-multiline

[Ihor Radchenko]

Jack Kamm <jackkamm@gmail.com> writes:

> This one might take some time to fix, since it's hard to reproduce and
> I'm not sure the cause of it. But here are 3 different solutions I am
> considering now:
>
> 1. In addition to printing `org-babel-python-eoe-indicator' after
>    execution, we could also print out a "beginning of execution"
>    indicator before execution, and then capture the output between the
>    beginning and end indicators. This is how the async session
>    execution works, and should avoid any possibility of capturing
>    prompts.

This idea looks interesting. Although I would not be so sure that it
will fix things - I have learned that comint has many edge cases we may
not easily anticipate.

For example, see the discussion in
https://yhetil.org/emacs-devel/87y1tgqhmc.fsf@localhost/

> 2. Instead of relying on our own custom `org-babel-python-send-string',
>    we could try switching to python.el's
>    `python-shell-send-string-no-output', which is probably more
>    robust. This would also allow removing the ugly
>    `org-babel-python-eoe-indicator' we currently print.

Beware non-standard REPL prompts. When we have something other than >>>,
the logic may have hard time figuring out output boundaries without
deterministic separators like eoe-indicator.

>    Downside is that the output would not be echoed into the session
>    anymore. To fix that, we could manually insert the captured output
>    into the comint session buffer after execution.  Alternatively, we
>    could add an argument to `python-shell-send-string-no-output' to
>    avoid suppressing output, submit it upstream to python.el, and then
>    backport to Org to support older emacs versions.

If we can (eventually) remove some custom code from Org and move it to
Emacs, it will be the best for working towards RMS request
https://orgmode.org/list/E1kIPh1-0001Lu-Rg@fencepost.gnu.org

> 3. Revisit a series of commits I made in 2020, which was supposed to
>    make session evaluation more robust, and was inspired by
>    `python-shell-send-string-no-output':
>    https://git.sr.ht/~bzg/org-mode/commit/4df12ea39
>
>    However, I had to partially revert that work, due to compatibility
>    issue with emacs 26.3:
>    https://list.orgmode.org/871rjcan53.fsf@kyleam.com/
>
>    I think we no longer support emacs 26.3, so I could potentially
>    revisit this now -- but it's been a few years and will take some
>    time to refresh my memory about this.

Note that I changed `org-babel-comint-wait-for-output' and some other
ob-comint machinery since that time. In particular, I tried to address
various edge cases we encountered with bash sessions. See the commentary
in `org-babel-comint-with-output'.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [BUG] FAILED test-ob-python/session-multiline

[Jack Kamm]

Ihor Radchenko <yantar92@posteo.net> writes:

> We have fairly regular CI test failures for one of the ob-python tests.
> The test does not fail _every_ time, but I keep seeing the problem in
> various Emacs versions, including Emacs 29.
>
> Example log: https://builds.sr.ht/~bzg/job/1047678#task-build
>
> In the test the result somehow includes prompt:
>
> Test test-ob-python/session-multiline condition:
>     (ert-test-failed
>      ((should
>        (equal "20"
> 	      (org-test-with-temp-text "#+begin_src python :session :results output\n  foo = 0\n  for _ in range(10):\n      foo += 1\n\n      foo += 1\n\n  print(foo)\n#+end_src" ...)))
>       :form
>       (equal "20" ">>> 20")
>       :value nil :explanation
>
>
> -->   (arrays-of-different-length 2 6 "20" ">>> 20" first-mismatch-at 0)))

Hello, sorry for the long time to address this.

I've just pushed a commit [1] that might address this, based on a new
hypothesis I have for the root cause:

https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=1eb598758980d5fa4d7bb21c98dfc56f42cae59a

Please let me know whether the problem continues, or whether it seems to
improve.

As an aside -- I am having a hard time figuring out how to monitor our
CI for this. When I search in

https://lists.sr.ht/~bzg/org-build-failures

I can only find an example from 11 months ago. The example you sent
(https://builds.sr.ht/~bzg/job/1047678#task-build) is more recent, but
is "Unlisted" and doesn't show up when I search for it.

Re: [BUG] FAILED test-ob-python/session-multiline

[Jack Kamm]

Ihor Radchenko <yantar92@posteo.net> writes:

>> 1. In addition to printing `org-babel-python-eoe-indicator' after
>>    execution, we could also print out a "beginning of execution"
>>    indicator before execution, and then capture the output between the
>>    beginning and end indicators. This is how the async session
>>    execution works, and should avoid any possibility of capturing
>>    prompts.
>
> This idea looks interesting. Although I would not be so sure that it
> will fix things - I have learned that comint has many edge cases we may
> not easily anticipate.
>
> For example, see the discussion in
> https://yhetil.org/emacs-devel/87y1tgqhmc.fsf@localhost/

I think this strategy could work better in ob-python than ob-shell
because ob-python sends code to a temp file and executes the whole file
at once, which should prevent prompts arising between commands.

I will probably try this approach next, if the fix I just sent here
doesn't work out:

https://list.orgmode.org/87h6mrihfg.fsf@gmail.com/

>>    Alternatively, we could add an argument to
>>    `python-shell-send-string-no-output' to avoid suppressing output,
>>    submit it upstream to python.el, and then backport to Org to
>>    support older emacs versions.
>
> If we can (eventually) remove some custom code from Org and move it to
> Emacs, it will be the best for working towards RMS request
> https://orgmode.org/list/E1kIPh1-0001Lu-Rg@fencepost.gnu.org

I started down this path here:

https://lists.gnu.org/archive/html/emacs-devel/2023-10/msg00004.html

But I haven't followed up because I started to have some doubts. In
particular, `python-shell-send-string-no-output' will terminate once it
detects a prompt, so if some output looks like it ends in a prompt then
it will terminate prematurely. Whereas in our current indicator-based
approach, the user accidentally emitting
`org-babel-python-eoe-indicator' is unlikely.

Another approach I have considered is to redirect sys.stdout from within
Python. In particular, set it to a custom class inheriting from IOBase
during the block's execution, that both prints and saves the output. I
think this approach could ultimately be more robust, and without needing
to print an ugly indicator token, but it could be complicated to do it
right.

Re: [BUG] FAILED test-ob-python/session-multiline

[Ihor Radchenko]

Jack Kamm <jackkamm@gmail.com> writes:

> Ihor Radchenko <yantar92@posteo.net> writes:
>
> ...
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=1eb598758980d5fa4d7bb21c98dfc56f42cae59a
>
> Please let me know whether the problem continues, or whether it seems to
> improve.

As soon as we fix CI :/ I think it is not working for the last month.

> As an aside -- I am having a hard time figuring out how to monitor our
> CI for this. When I search in
>
> https://lists.sr.ht/~bzg/org-build-failures
>
> I can only find an example from 11 months ago. The example you sent
> (https://builds.sr.ht/~bzg/job/1047678#task-build) is more recent, but
> is "Unlisted" and doesn't show up when I search for it.

The most recent report in https://lists.sr.ht/~bzg/org-build-failures is
https://lists.sr.ht/~bzg/org-build-failures/%3CCVFCM84I774J.3RMTMSL25EMW2%40cirno2%3E
linking to https://builds.sr.ht/~bzg/job/1055574 where the failing test
is FAILED  test-ob-python/session-multiline

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [BUG] FAILED test-ob-python/session-multiline

[Ihor Radchenko]

Jack Kamm <jackkamm@gmail.com> writes:

>> If we can (eventually) remove some custom code from Org and move it to
>> Emacs, it will be the best for working towards RMS request
>> https://orgmode.org/list/E1kIPh1-0001Lu-Rg@fencepost.gnu.org
>
> I started down this path here:
>
> https://lists.gnu.org/archive/html/emacs-devel/2023-10/msg00004.html
>
> But I haven't followed up because I started to have some doubts. In
> particular, `python-shell-send-string-no-output' will terminate once it
> detects a prompt, so if some output looks like it ends in a prompt then
> it will terminate prematurely. Whereas in our current indicator-based
> approach, the user accidentally emitting
> `org-babel-python-eoe-indicator' is unlikely.

This problem is not new - it exist in comint.el itself.
I worked around it in ob-shell by re-defining prompt to be unique - see
`org-babel-shell-set-prompt-commands'. Of course, it is not a universal
solution.

> Another approach I have considered is to redirect sys.stdout from within
> Python. In particular, set it to a custom class inheriting from IOBase
> during the block's execution, that both prints and saves the output. I
> think this approach could ultimately be more robust, and without needing
> to print an ugly indicator token, but it could be complicated to do it
> right.

It may work, although it will affect certain workflows. I know that some
people use a mix of babel session blocks and manual comint buffer
interaction where they type commands manually. Hiding the output may not
be ideal in such scenario.

That said, the described scenario is somewhat buggy already.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [BUG] FAILED test-ob-python/session-multiline

[Ihor Radchenko]

Ihor Radchenko <yantar92@posteo.net> writes:

> Jack Kamm <jackkamm@gmail.com> writes:
>
>> Ihor Radchenko <yantar92@posteo.net> writes:
>>
>> ...
>> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=1eb598758980d5fa4d7bb21c98dfc56f42cae59a
>>
>> Please let me know whether the problem continues, or whether it seems to
>> improve.
>
> As soon as we fix CI :/ I think it is not working for the last month.

I am no longer seeing the failures.
Fixed.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>