Re: [O] [patch][babel] `org-babel-result-end' bug fix and regression tests
Applied, Thanks, Martyn Jago writes: > Hi Eric > > Eric Schulte writes: > > > [...] > >>> >>> All the tests supplied with the exception of >>> `test-ob/org-babel-remove-result--results-raw' will still pass without >>> the change. >>> >> >> Alright, would you be willing to resubmit the patch including only those >> tests which should still apply? This results handling is certainly an >> area which will benefit from beefing up the test suite. > > > The attached patch contains the remaining tests. > > Best, Martyn > > > -- Eric Schulte http://cs.unm.edu/~eschulte/
Re: [O] [patch][babel] `org-babel-result-end' bug fix and regression tests
Hi Eric Eric Schulte writes: [...] >> >> All the tests supplied with the exception of >> `test-ob/org-babel-remove-result--results-raw' will still pass without >> the change. >> > > Alright, would you be willing to resubmit the patch including only those > tests which should still apply? This results handling is certainly an > area which will benefit from beefing up the test suite. The attached patch contains the remaining tests. Best, Martyn >From 6700e3f350c76f68891ad0ccd35538b5523312d9 Mon Sep 17 00:00:00 2001 From: Martyn Jago Date: Fri, 6 Jan 2012 19:36:28 + Subject: [PATCH] Regression tests regarding code block results and result removal/replacement. * testing/lisp/test-ob.el: Regression tests regarding code block results and result removal/replacement --- testing/lisp/test-ob.el | 205 +-- 1 files changed, 198 insertions(+), 7 deletions(-) diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el index dac6866..64e261a 100644 --- a/testing/lisp/test-ob.el +++ b/testing/lisp/test-ob.el @@ -137,13 +137,7 @@ #+name: i-have-a-name #+begin_src emacs-lisp 42 -#+end_src - -#+name: -: 42 - -#+name: i-have-a-name -: 42" +#+end_src" (progn (org-babel-next-src-block 1) @@ -671,6 +665,203 @@ on two lines (org-babel-balanced-split ":a 1 :b [2 3] :c (4 :d (5 6))" '((32 9) . 58) +(ert-deftest test-ob/commented-last-block-line-no-var () + (org-test-with-temp-text-in-file " +#+begin_src emacs-lisp +;; +#+end_src" +(progn + (org-babel-next-src-block) + (org-ctrl-c-ctrl-c) + (should (re-search-forward "\\#\\+results:" nil t)) + (forward-line) + (should + (string= + "" + (buffer-substring-no-properties (point-at-bol) (point-at-eol)) + (org-test-with-temp-text-in-file " +#+begin_src emacs-lisp +\"some text\";; +#+end_src" + +(progn + (org-babel-next-src-block) + (org-ctrl-c-ctrl-c) + (should (re-search-forward "\\#\\+results:" nil t)) + (forward-line) + (should + (string= + ": some text" + (buffer-substring-no-properties (point-at-bol) (point-at-eol))) + +(ert-deftest test-ob/commented-last-block-line-with-var () + (org-test-with-temp-text-in-file " +#+begin_src emacs-lisp :var a=1 +;; +#+end_src" +(progn + (org-babel-next-src-block) + (org-ctrl-c-ctrl-c) + (re-search-forward "\\#\\+results:" nil t) + (forward-line) + (should (string= + "" + (buffer-substring-no-properties (point-at-bol) (point-at-eol)) + (org-test-with-temp-text-in-file " +#+begin_src emacs-lisp :var a=2 +2;; +#+end_src" +(progn + (org-babel-next-src-block) + (org-ctrl-c-ctrl-c) + (re-search-forward "\\#\\+results:" nil t) + (forward-line) + (should (string= + ": 2" + (buffer-substring-no-properties (point-at-bol) (point-at-eol))) + +(defun test-ob-verify-result-and-removed-result (result buffer-text) + "Test helper function to test `org-babel-remove-result'. +A temp buffer is populated with BUFFER-TEXT, the first block is executed, +and the result of execution is verified against RESULT. + +The block is actually executed /twice/ to ensure result +replacement happens correctly." + (org-test-with-temp-text + buffer-text +(progn + (org-babel-next-src-block) (org-ctrl-c-ctrl-c) (org-ctrl-c-ctrl-c) + (should (re-search-forward "\\#\\+results:" nil t)) + (forward-line) + (should (string= result + (buffer-substring-no-properties + (point-at-bol) + (- (point-max) 16 + (org-babel-previous-src-block) (org-babel-remove-result) + (should (string= buffer-text + (buffer-substring-no-properties + (point-min) (point-max))) + +(ert-deftest test-ob/org-babel-remove-result--results-default () + "Test `org-babel-remove-result' with default :results." + (mapcar (lambda (language) + (test-ob-verify-result-and-removed-result + "\n" + (concat +"* org-babel-remove-result +#+begin_src " language " +#+end_src + +* next heading"))) + '("sh" "emacs-lisp"))) + +(ert-deftest test-ob/org-babel-remove-result--results-list () + "Test `org-babel-remove-result' with :results list." + (test-ob-verify-result-and-removed-result + "- 1 +- 2 +- 3 +- (quote (4 5))" + +"* org-babel-remove-result +#+begin_src emacs-lisp :results list +'(1 2 3 '(4 5)) +#+end_src + +* next heading")) + +;; TODO FIXME Activate when Eric's trailing newline fix has been committed +;; (ert-deftest test-ob/org-babel-remove-result--results-wrap () +;; (test-ob-verify-result-and-removed-result +;;":RESULTS: +;; hello there +;; :END:" +;; +;; "* org-babel-remove-result +;; +;; +begin_src emacs-lisp :results wrap +;; \"hello there\" +;; #+end_src +;; +;; * next heading")) + +(ert-deftest test-ob/org-babel-remove-result--results-org () + "Test `org-babel-remove-result' with :results org." + (test-ob-verify-result-and-removed-result +
Re: [O] [patch][babel] `org-babel-result-end' bug fix and regression tests
Martyn Jago writes: > Eric Schulte writes: > Hi Eric > >> Hi Martyn, >> >> Unfortunately there is no way to remove raw results because there is no >> way to know where the results end. While your patch will certainly work >> most of the time, it will not work in cases where the results includes >> an empty line, and ultimately I think any attempt to remove raw results >> will result in confusion. > > Yes I appreciate that would be a problem if there were empty lines. I just > thought that most of the time is better than none of the time, but I > understand your choice. Certainly wrap works fine. > > I have noticed one small issue regarding :results wrap, which is that an > extra newline is appended to the end of the result each time > `org-babel-execute-src-block' is executed. I imagine this can be safely > removed? > Yes, this should be fixed. These newline-append issues are tricky as it can be difficult to fix for one type of result without accidentally breaking for other types of results. > > There are a few other minor issues also with `org-babel-remove-results' > that probably should be fixed at some time (regarding append and > prepend). > >> I think this patch should not be applied (although maybe some of the >> test cases could still be useful). > > All the tests supplied with the exception of > `test-ob/org-babel-remove-result--results-raw' will still pass without > the change. > Alright, would you be willing to resubmit the patch including only those tests which should still apply? This results handling is certainly an area which will benefit from beefing up the test suite. Thanks, > > Best, Martyn > > -- Eric Schulte http://cs.unm.edu/~eschulte/
Re: [O] [patch][babel] `org-babel-result-end' bug fix and regression tests
Eric Schulte writes: Hi Eric > Hi Martyn, > > Unfortunately there is no way to remove raw results because there is no > way to know where the results end. While your patch will certainly work > most of the time, it will not work in cases where the results includes > an empty line, and ultimately I think any attempt to remove raw results > will result in confusion. Yes I appreciate that would be a problem if there were empty lines. I just thought that most of the time is better than none of the time, but I understand your choice. Certainly wrap works fine. I have noticed one small issue regarding :results wrap, which is that an extra newline is appended to the end of the result each time `org-babel-execute-src-block' is executed. I imagine this can be safely removed? There are a few other minor issues also with `org-babel-remove-results' that probably should be fixed at some time (regarding append and prepend). > I think this patch should not be applied (although maybe some of the > test cases could still be useful). All the tests supplied with the exception of `test-ob/org-babel-remove-result--results-raw' will still pass without the change. Best, Martyn
Re: [O] [patch][babel] `org-babel-result-end' bug fix and regression tests
Hi Martyn, Unfortunately there is no way to remove raw results because there is no way to know where the results end. While your patch will certainly work most of the time, it will not work in cases where the results includes an empty line, and ultimately I think any attempt to remove raw results will result in confusion. If removable raw results are desired then the ":results wrap" option may be used. I believe this is mentioned in the manual (if not it should be). I think this patch should not be applied (although maybe some of the test cases could still be useful). Thanks, Martyn Jago writes: > `org-babel-result-end' bug fix and `org-babel-remove-result' regression tests. > > * lisp/ob.el: > > The code block below will currently act as though :results prepend > is set. This is due to `org-babel-result-end' being unable to > find the correct end of a raw result. This patch fixes that. > > #+begin_src emacs-lisp :results raw > "a line" > #+end_src > > #+results: > a line > a line > > * testing/lisp/test-ob.el: > > Several regression tests that test the correct (multiple) execution of > code blocks in the various results formats. The tests also test that > 'org-babel-remove-result' correctly removes the result. > > Best, Martyn > > -- Eric Schulte http://cs.unm.edu/~eschulte/