* [PATCH 2/2] test-ob-emacs-lisp: Test :lexical src block header argument
@ 2019-03-14 10:25 Sebastian Miele
2019-03-14 14:58 ` Nicolas Goaziou
0 siblings, 1 reply; 5+ messages in thread
From: Sebastian Miele @ 2019-03-14 10:25 UTC (permalink / raw)
To: emacs-orgmode
* testing/lisp/test-ob-emacs-lisp.el
(test-ob-emacs-lisp-dynamic-lexical-text,
test-ob-emacs-lisp-dynamic-lexical-expr,
ob-emacs-lisp/dynamic-lexical-execute,
ob-emacs-lisp/dynamic-lexical-edit): Add tests that check the
correct handling of the :lexical header argument when executing
source blocks and when creating editing buffers for source blocks.
---
testing/lisp/test-ob-emacs-lisp.el | 86 ++++++++++++++++++++++++++++++
1 file changed, 86 insertions(+)
diff --git a/testing/lisp/test-ob-emacs-lisp.el b/testing/lisp/test-ob-emacs-lisp.el
index 078cad988..a48f0c7dd 100644
--- a/testing/lisp/test-ob-emacs-lisp.el
+++ b/testing/lisp/test-ob-emacs-lisp.el
@@ -76,6 +76,92 @@
(buffer-substring-no-properties (line-beginning-position 2)
(line-end-position 2))))))
+(defun test-ob-emacs-lisp-dynamic-lexical-text (expr lexical)
+ (concat "\n"
+ "#+begin_src emacs-lisp :lexical " lexical " :results verbatim\n"
+ (format "%S" expr) "\n"
+ "#+end_src"))
+
+(defvar test-ob-emacs-lisp-dynamic-lexical-expr
+ '(let ((x 'dynamic))
+ (funcall
+ (let ((x 'lexical))
+ (lambda () x)))))
+
+(ert-deftest ob-emacs-lisp/dynamic-lexical-execute ()
+ (cl-flet ((execute (text)
+ (org-test-with-temp-text-in-file text
+ (org-babel-next-src-block)
+ (org-babel-execute-maybe)
+ (re-search-forward "results" nil t)
+ (re-search-forward ": " nil t)
+ (buffer-substring-no-properties (point) (point-at-eol)))))
+ ;;
+ (cl-flet ((text (lexical)
+ (test-ob-emacs-lisp-dynamic-lexical-text
+ test-ob-emacs-lisp-dynamic-lexical-expr
+ lexical)))
+ (should (string= "dynamic" (execute (text "no" ))))
+ (should (string= "lexical" (execute (text "yes")))))
+ ;;
+ (cl-flet ((text (lexical)
+ (test-ob-emacs-lisp-dynamic-lexical-text
+ 'x
+ lexical)))
+ (should (string= "dynamic" (let ((x 'dynamic))
+ (execute (text "no")))))
+ (should (string= "lexical" (execute (text "'((x . lexical))")))))
+ ;;
+ ;; Src block execution uses `eval'. `eval' does not dynamically
+ ;; bind `lexical-binding' to the value of its LEXICAL
+ ;; parameter. Hence, (eval 'lexical-binding LEXICAL) evaluates to
+ ;; the same value that just `lexical-binding' evaluates to, no
+ ;; matter what is given as the LEXICAL parameter to eval. So the
+ ;; following does not work as intended:
+ ;;
+ ;;(cl-flet ((text (lexical)
+ ;; (test-ob-emacs-lisp-dynamic-lexical-text
+ ;; 'lexical-binding
+ ;; lexical)))
+ ;; (should (string= "nil" (execute (text "no" ))))
+ ;; (should (string= "t" (execute (text "yes" ))))
+ ;; (should (string= "((x . 0))" (execute (text "'((x . 0))")))))
+ ))
+
+(ert-deftest ob-emacs-lisp/dynamic-lexical-edit ()
+ (cl-flet ((execute (text)
+ (org-test-with-temp-text-in-file text
+ (org-babel-next-src-block)
+ (org-edit-src-code)
+ (goto-char (point-max))
+ (prog1 (eval-last-sexp 0)
+ (org-edit-src-exit)))))
+ ;;
+ (cl-flet ((text (lexical)
+ (test-ob-emacs-lisp-dynamic-lexical-text
+ test-ob-emacs-lisp-dynamic-lexical-expr
+ lexical)))
+ (should (eq 'dynamic (execute (text "no" ))))
+ (should (eq 'lexical (execute (text "yes")))))
+ ;;
+ (cl-flet ((text (lexical)
+ (test-ob-emacs-lisp-dynamic-lexical-text
+ 'x
+ lexical)))
+ (should (eq 'dynamic (let ((x 'dynamic))
+ (execute (text "no")))))
+ (should (eq 'lexical (execute (text "'((x . lexical))")))))
+ ;;
+ (cl-flet ((text (lexical)
+ (test-ob-emacs-lisp-dynamic-lexical-text
+ 'lexical-binding
+ lexical)))
+ (should (equal nil (execute (text "no" ))))
+ (should (equal t (execute (text "yes" ))))
+ (should (equal '((x . 0)) (execute (text "'((x . 0))")))))
+ ))
+
+
(provide 'test-ob-emacs-lisp)
;;; test-ob-emacs-lisp.el ends here
--
2.21.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] test-ob-emacs-lisp: Test :lexical src block header argument
2019-03-14 10:25 [PATCH 2/2] test-ob-emacs-lisp: Test :lexical src block header argument Sebastian Miele
@ 2019-03-14 14:58 ` Nicolas Goaziou
2019-03-14 19:28 ` Sebastian Miele
0 siblings, 1 reply; 5+ messages in thread
From: Nicolas Goaziou @ 2019-03-14 14:58 UTC (permalink / raw)
To: Sebastian Miele; +Cc: emacs-orgmode
Hello,
Sebastian Miele <sebastian.miele@gmail.com> writes:
> * testing/lisp/test-ob-emacs-lisp.el
> (test-ob-emacs-lisp-dynamic-lexical-text,
> test-ob-emacs-lisp-dynamic-lexical-expr,
> ob-emacs-lisp/dynamic-lexical-execute,
> ob-emacs-lisp/dynamic-lexical-edit): Add tests that check the
> correct handling of the :lexical header argument when executing
> source blocks and when creating editing buffers for source blocks.
Thank you.
<nitpick>
However, your tests are very convoluted. It is better than no test, but
if, unfortunately, one of them fail in some distant future, it may take
more time understanding what happens in the test than actually fixing
the bug.
Would you mind rewriting them with simple macros like, e.g.,
`org-test-with-temp-text', and use as little helper functions as
possible? IMO, code repetition in tests is not a problem.
</nitpick>
Regards,
--
Nicolas Goaziou
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] test-ob-emacs-lisp: Test :lexical src block header argument
2019-03-14 14:58 ` Nicolas Goaziou
@ 2019-03-14 19:28 ` Sebastian Miele
2019-03-14 22:38 ` Nicolas Goaziou
0 siblings, 1 reply; 5+ messages in thread
From: Sebastian Miele @ 2019-03-14 19:28 UTC (permalink / raw)
To: Nicolas Goaziou; +Cc: emacs-orgmode
Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:
> [...]
>
> <nitpick>
> However, your tests are very convoluted. It is better than no test, but
> if, unfortunately, one of them fail in some distant future, it may take
> more time understanding what happens in the test than actually fixing
> the bug.
>
> Would you mind rewriting them with simple macros like, e.g.,
> `org-test-with-temp-text', and use as little helper functions as
> possible? IMO, code repetition in tests is not a problem.
> </nitpick>
After some initial tooth-grinding, I did rewrite them, and actually like
the result more than the previous version. Thank you for the suggestion!
Here is the updated patch:
* testing/lisp/test-ob-emacs-lisp.el
(ob-emacs-lisp/dynamic-lexical-execute,
ob-emacs-lisp/dynamic-lexical-edit): Add tests that check the
correct handling of the :lexical header argument when executing
source blocks and when creating editing buffers for source blocks.
---
testing/lisp/test-ob-emacs-lisp.el | 89 ++++++++++++++++++++++++++++++
1 file changed, 89 insertions(+)
diff --git a/testing/lisp/test-ob-emacs-lisp.el b/testing/lisp/test-ob-emacs-lisp.el
index 078cad988..24a373f86 100644
--- a/testing/lisp/test-ob-emacs-lisp.el
+++ b/testing/lisp/test-ob-emacs-lisp.el
@@ -76,6 +76,95 @@
(buffer-substring-no-properties (line-beginning-position 2)
(line-end-position 2))))))
+(ert-deftest ob-emacs-lisp/dynamic-lexical-execute ()
+ (cl-flet ((execute (text)
+ (org-test-with-temp-text-in-file text
+ (org-babel-next-src-block)
+ (org-babel-execute-maybe)
+ (re-search-forward "results" nil t)
+ (re-search-forward ": " nil t)
+ (buffer-substring-no-properties (point) (point-at-eol)))))
+
+ (should (string= "dynamic" (execute "
+#+begin_src emacs-lisp :lexical no :results verbatim
+(let ((x 'dynamic)) (funcall (let ((x 'lexical)) (lambda () x))))
+#+end_src")))
+
+ (should (string= "lexical" (execute "
+#+begin_src emacs-lisp :lexical yes :results verbatim
+(let ((x 'dynamic)) (funcall (let ((x 'lexical)) (lambda () x))))
+#+end_src")))
+
+ (should (string= "dynamic" (let ((x 'dynamic)) (execute "
+#+begin_src emacs-lisp :lexical no :results verbatim
+x
+#+end_src"))))
+
+ (should (string= "lexical" (let ((x 'dynamic)) (execute "
+#+begin_src emacs-lisp :lexical '((x . lexical)) :results verbatim
+x
+#+end_src"))))
+
+ ;; Src block execution uses `eval'. As of 2019-02-26, `eval' does
+ ;; not dynamically bind `lexical-binding' to the value of its
+ ;; LEXICAL parameter. Hence, (eval 'lexical-binding LEXICAL)
+ ;; evaluates to the same value that just `lexical-binding'
+ ;; evaluates to, even if LEXICAL is different. So tests like the
+ ;; following do not work here:
+ ;;
+ ;; (should (string= "t" (execute "
+ ;; #+begin_src emacs-lisp :lexical yes :results verbatim
+ ;; lexical-binding
+ ;; #+end_src")))
+ ;;
+ ;; However, the corresponding test in
+ ;; `ob-emacs-lisp/dynamic-lexical-edit' does work.
+ ))
+
+(ert-deftest ob-emacs-lisp/dynamic-lexical-edit ()
+ (cl-flet ((execute (text)
+ (org-test-with-temp-text-in-file text
+ (org-babel-next-src-block)
+ (org-edit-src-code)
+ (goto-char (point-max))
+ (prog1 (eval-last-sexp 0)
+ (org-edit-src-exit)))))
+
+ (should (eq 'dynamic (execute "
+#+begin_src emacs-lisp :lexical no :results verbatim
+(let ((x 'dynamic)) (funcall (let ((x 'lexical)) (lambda () x))))
+#+end_src")))
+
+ (should (eq 'lexical (execute "
+#+begin_src emacs-lisp :lexical yes :results verbatim
+(let ((x 'dynamic)) (funcall (let ((x 'lexical)) (lambda () x))))
+#+end_src")))
+
+ (should (eq 'dynamic (let ((x 'dynamic)) (execute "
+#+begin_src emacs-lisp :lexical no :results verbatim
+x
+#+end_src"))))
+
+ (should (eq 'lexical (let ((x 'dynamic)) (execute "
+#+begin_src emacs-lisp :lexical '((x . lexical)) :results verbatim
+x
+#+end_src"))))
+
+ (should (equal nil (execute "
+#+begin_src emacs-lisp :lexical no :results verbatim
+lexical-binding
+#+end_src")))
+
+ (should (equal t (execute "
+#+begin_src emacs-lisp :lexical yes :results verbatim
+lexical-binding
+#+end_src")))
+
+ (should (equal '((x . 0)) (execute "
+#+begin_src emacs-lisp :lexical '((x . 0)) :results verbatim
+lexical-binding
+#+end_src")))))
+
(provide 'test-ob-emacs-lisp)
;;; test-ob-emacs-lisp.el ends here
--
2.21.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] test-ob-emacs-lisp: Test :lexical src block header argument
2019-03-14 19:28 ` Sebastian Miele
@ 2019-03-14 22:38 ` Nicolas Goaziou
2019-03-15 19:28 ` Sebastian Miele
0 siblings, 1 reply; 5+ messages in thread
From: Nicolas Goaziou @ 2019-03-14 22:38 UTC (permalink / raw)
To: Sebastian Miele; +Cc: emacs-orgmode
Sebastian Miele <sebastian.miele@gmail.com> writes:
> After some initial tooth-grinding,
My intent is not to be negative. I do understand that writing tests
takes time.
However, from experience, having to fix a regression when your only
indication comes from a test you do not fully understand is a pain. To
see what I mean, have a look at tests in
"test-ob-header-arg-defaults.el" and imagine one of them fails…
> I did rewrite them, and actually like
> the result more than the previous version. Thank you for the suggestion!
>
> Here is the updated patch:
Thank you. I applied it.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] test-ob-emacs-lisp: Test :lexical src block header argument
2019-03-14 22:38 ` Nicolas Goaziou
@ 2019-03-15 19:28 ` Sebastian Miele
0 siblings, 0 replies; 5+ messages in thread
From: Sebastian Miele @ 2019-03-15 19:28 UTC (permalink / raw)
To: Nicolas Goaziou; +Cc: emacs-orgmode
Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:
> However, from experience, having to fix a regression when your only
> indication comes from a test you do not fully understand is a pain. To
> see what I mean, have a look at tests in
> "test-ob-header-arg-defaults.el" and imagine one of them fails…
After looking into test-ob-header-arg-defaults.el and its accompanying
Org file for two hours I still do not understand everything that's going
on there. You are right. And I now have a solid desire and strategy to
circumnavigate the production of such problems in the future. Thank you
for the input.
Best wishes!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-03-15 19:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-14 10:25 [PATCH 2/2] test-ob-emacs-lisp: Test :lexical src block header argument Sebastian Miele
2019-03-14 14:58 ` Nicolas Goaziou
2019-03-14 19:28 ` Sebastian Miele
2019-03-14 22:38 ` Nicolas Goaziou
2019-03-15 19:28 ` Sebastian Miele
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).