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