[PATCH 3/3] test: Add address cleaning tests.
Dmitry Kurochkin
dmitry.kurochkin at gmail.com
Mon Jan 23 09:26:54 PST 2012
On Thu, 19 Jan 2012 12:54:03 +0000, David Edmondson <dme at dme.org> wrote:
> Including some more test framework in test-lib.el.
IMO test-lib.el changes should be in a separate patch.
> ---
> test/emacs-address-cleaning.el | 29 +++++++++++++++++++++++++++++
> test/emacs-address-cleaning.sh | 12 ++++++++++++
> test/notmuch-test | 1 +
> test/test-lib.el | 30 ++++++++++++++++++++++++++++++
> 4 files changed, 72 insertions(+), 0 deletions(-)
> create mode 100644 test/emacs-address-cleaning.el
> create mode 100755 test/emacs-address-cleaning.sh
>
> diff --git a/test/emacs-address-cleaning.el b/test/emacs-address-cleaning.el
> new file mode 100644
> index 0000000..59e8d92
> --- /dev/null
> +++ b/test/emacs-address-cleaning.el
> @@ -0,0 +1,29 @@
> +(defun notmuch-test-address-cleaning-1 ()
> + (notmuch-test-compare (notmuch-show-clean-address "dme at dme.org")
> + "dme at dme.org"))
> +
> +(defun notmuch-test-address-cleaning-2 ()
> + (let* ((input '("foo at bar.com"
> + "<foo at bar.com>"
> + "Foo Bar <foo at bar.com>"
> + "foo at bar.com <foo at bar.com>"
> + "\"Foo Bar\" <foo at bar.com>"))
> + (expected '("foo at bar.com"
> + "foo at bar.com"
> + "Foo Bar <foo at bar.com>"
> + "foo at bar.com"
> + "Foo Bar <foo at bar.com>"))
> + (output (mapcar #'notmuch-show-clean-address input)))
> + (notmuch-test-compare output expected)))
> +
> +(defun notmuch-test-address-cleaning-3 ()
> + (let* ((input '("ДБ <db-uknot at stop.me.uk>"
> + "foo (at home) <foo at bar.com>"
> + "foo [at home] <foo at bar.com>"
> + "Foo Bar"))
> + (expected '("ДБ <db-uknot at stop.me.uk>"
> + "foo (at home) <foo at bar.com>"
> + "foo [at home] <foo at bar.com>"
> + "Foo Bar"))
> + (output (mapcar #'notmuch-show-clean-address input)))
> + (notmuch-test-compare output expected)))
At a glance, the tests look good. Though more review of the tests
itself would be nice.
> diff --git a/test/emacs-address-cleaning.sh b/test/emacs-address-cleaning.sh
> new file mode 100755
> index 0000000..1a6eff5
> --- /dev/null
> +++ b/test/emacs-address-cleaning.sh
> @@ -0,0 +1,12 @@
> +#!/usr/bin/env bash
> +
> +test_description="emacs address cleaning"
> +. test-lib.sh
> +
> +for i in 1 2 3; do
> + test_begin_subtest "notmuch-test-address-clean-$i"
> + test_emacs_expect_t \
> + '(load "emacs-address-cleaning.el") (notmuch-test-address-cleaning-'$i')'
> +done
> +
> +test_done
> diff --git a/test/notmuch-test b/test/notmuch-test
> index d034f99..3f1740c 100755
> --- a/test/notmuch-test
> +++ b/test/notmuch-test
> @@ -53,6 +53,7 @@ TESTS="
> hooks
> argument-parsing
> emacs-test-functions.sh
> + emacs-address-cleaning.sh
> "
> TESTS=${NOTMUCH_TESTS:=$TESTS}
>
> diff --git a/test/test-lib.el b/test/test-lib.el
> index 1d51b8d..033270d 100644
> --- a/test/test-lib.el
> +++ b/test/test-lib.el
> @@ -20,6 +20,8 @@
> ;;
> ;; Authors: Dmitry Kurochkin <dmitry.kurochkin at gmail.com>
>
> +(require 'cl) ;; This code is generally used uncompiled.
> +
> ;; `read-file-name' by default uses `completing-read' function to read
> ;; user input. It does not respect `standard-input' variable which we
> ;; use in tests to provide user input. So replace it with a plain
> @@ -77,6 +79,7 @@ nothing."
> (add-hook-counter 'notmuch-hello-mode-hook)
> (add-hook-counter 'notmuch-hello-refresh-hook)
>
> +
Please revert.
> (defmacro notmuch-test-run-test (&rest body)
> "Evaluate a BODY of test expressions and output the result."
> `(with-temp-buffer
> @@ -85,3 +88,30 @@ nothing."
> result
> (prin1-to-string result)))
> (test-output))))
> +
> +;; Functions to help when writing tests:
> +
IMO this comment is misplaced. There are other functions above which
also help to write tests e.g. `test-output. I think we should either
remove the comment or move all test helpers in one place below the
comment (in a separate patch).
> +(defun notmuch-test-reporter (output expected)
Please consider renaming to `notmuch-test-report-unexpected'.
> + "Report that the `output' does not match the `expected' result."
> + (concat "Expect:\t" (prin1-to-string expected) "\n"
> + "Output:\t" (prin1-to-string output) "\n"))
> +
> +(defun notmuch-test-unsimilar (output expected)
Please consider renaming to notmuch-test-unexpected.
> + "`output' and `expected' are dissimilar. Show a summary of
Previous line says "unsimilar", now it is "dissimilar". I am not sure
which one is correct. Also, it is not clear what "similar" means for
test results. IMO "not equal" would be more clear.
> +the differences, ignoring similarities."
> + (cond ((and (listp output)
> + (listp expected))
> + (apply #'concat (loop for o in output
> + for e in expected
> + if (not (equal o e))
> + collect (notmuch-test-reporter o e))))
> +
> + (t
> + ;; Catch all case.
> + (notmuch-test-reporter output expected))))
> +
> +(defun notmuch-test-compare (output expected)
Please consider renaming to notmuch-test-expect-equal for consistency
with shell tests.
> + "Compare `output' with `expected'. Report any discrepencies."
> + (if (equal output expected)
> + t
> + (notmuch-test-unsimilar output expected)))
I do not like that equality check is split between notmuch-test-compare
and notmuch-test-unsimilar. It makes the code harder to read (took me a
while to understand why "catch all case" above does not need the
equality check). Also, notmuch-test-unsimilar would be hard to reuse
because the comparison check is done in one case and not done in
another. How about merging them? The code would look something like:
(cond
(both are lists)
; code from notmuch-test-unsimilar
(catch all case)
(if not equal)
(notmuch-test-report-unexpected output expected))
Regards,
Dmitry
> --
> 1.7.8.3
>
> _______________________________________________
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
More information about the notmuch
mailing list