From: Ihor Radchenko <yantar92@posteo.net>
To: Leo Butler <Leo.Butler@umanitoba.ca>
Cc: Org Mode Mailing List <emacs-orgmode@gnu.org>
Subject: Re: [PATCH] lisp/ob-octave.el, was [PATCH] rfc: using ert-deftest with side-effects
Date: Wed, 09 Nov 2022 05:14:45 +0000 [thread overview]
Message-ID: <8735asbtfe.fsf@localhost> (raw)
In-Reply-To: <87zgd1b4rn.fsf_-_@t14.reltub.ca>
Leo Butler <Leo.Butler@umanitoba.ca> writes:
> Ihor,
> Thanks for your feeback and the pointer. I have revised the tests and
> attach the revised patch.
Thanks!
Note that your patch is over 15LOC, which exceeds legally allowed
contribution size for people without copyright assignment.
Would you be interested to sign the copyright assignment form and send
it to FSF? See https://orgmode.org/worg/org-contribute.html#copyright
for details. The process usually takes a few days on FSF side (they are
obliged to reply within 5 working days).
Below are some comments.
> * testing/lisp/test-ob-octave.el:
>
> Add the tests ob-octave/graphics-file and
> ob-octave/graphics-file-session. The first test verifies that the
Please use double space " " between sentences. See
https://orgmode.org/worg/org-contribute.html#commit-messages
> - (format "print -dpng %s" gfx-file))
> + (format "print -dpng %s\nans=%S" gfx-file gfx-file))
Is there any reason why %S but not %s?
> * Graphical tests
> -#+begin_src octave :results graphics :file chart.png
> +
> +Graphics file. This test is performed by =ob-octave/graphics-file= in =testing/lisp/test-ob-octave.el=.
By convention, we use double space in distributed Org files and ~code~
for symbol markup. See doc/Documentation_Standards.org.
(It is not strictly necessary here, but would be nice to be consistent)
> + (org-babel-execute-src-block)
> + (should (search-forward (format "[[file:%s]]" file) nil nil))
> + (should (file-readable-p file))
> + (should (let ((size (nth 7 (file-attributes file))))
It would be more clear to use `file-attribute-size' instead of `nth'.
> + (> size 0)))
> + (should (not (get-buffer "*Org-Babel Error Output*"))))
`should-not' would be a bit more succinct.
> + (should (let ((size (nth 7 (file-attributes file))))
> + (> size 0)))
> + (should (not (get-buffer "*Org-Babel Error Output*"))))
See the previous comment.
--
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>
next prev parent reply other threads:[~2022-11-09 5:14 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-07 17:03 [PATCH] rfc: using ert-deftest with side-effects Leo Butler
2022-11-08 7:40 ` Ihor Radchenko
2022-11-08 19:55 ` [PATCH] lisp/ob-octave.el, was " Leo Butler
2022-11-09 5:14 ` Ihor Radchenko [this message]
2022-11-09 20:33 ` Leo Butler
2022-11-14 1:56 ` Ihor Radchenko
2022-11-15 19:43 ` Leo Butler
2022-12-17 8:25 ` Ihor Radchenko
2022-12-17 10:06 ` Ihor Radchenko
2022-12-21 11:56 ` Ihor Radchenko
2022-12-22 13:32 ` Leo Butler
2022-12-27 14:45 ` Ihor Radchenko
2022-12-29 9:54 ` Ihor Radchenko
2023-01-02 8:42 ` Ihor Radchenko
2023-01-05 20:08 ` Leo Butler
2023-01-06 15:11 ` Ihor Radchenko
2023-01-07 3:08 ` Leo Butler
2023-01-10 20:30 ` Leo Butler
2023-01-11 11:15 ` Ihor Radchenko
2023-01-11 21:51 ` Leo Butler
2023-01-12 8:42 ` Ihor Radchenko
2023-01-13 18:00 ` Ihor Radchenko
2023-01-14 13:04 ` Leo Butler
2023-01-14 13:13 ` Ihor Radchenko
2023-01-14 15:16 ` Max Nikulin
2023-01-23 10:32 ` Ihor Radchenko
2023-01-24 18:08 ` Leo Butler
2023-01-07 12:48 ` Ihor Radchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.orgmode.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8735asbtfe.fsf@localhost \
--to=yantar92@posteo.net \
--cc=Leo.Butler@umanitoba.ca \
--cc=emacs-orgmode@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).