From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolas Goaziou Subject: Re: [PATCH 1/2] Add tests for org-refile-get-targets Date: Mon, 15 May 2017 18:38:09 +0200 Message-ID: <87efvqxeha.fsf@nicolasgoaziou.fr> References: <20170515125455.18251-1-seb@wirrsal.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:33425) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dAJ0j-00068E-SK for emacs-orgmode@gnu.org; Mon, 15 May 2017 12:38:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dAJ0g-0006oo-Nc for emacs-orgmode@gnu.org; Mon, 15 May 2017 12:38:17 -0400 Received: from relay7-d.mail.gandi.net ([217.70.183.200]:37754) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dAJ0g-0006oO-HS for emacs-orgmode@gnu.org; Mon, 15 May 2017 12:38:14 -0400 In-Reply-To: <20170515125455.18251-1-seb@wirrsal.net> ("Sebastian =?utf-8?Q?Reu=C3=9Fe=22's?= message of "Mon, 15 May 2017 14:54:54 +0200") List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: "Emacs-orgmode" To: Sebastian =?utf-8?Q?Reu=C3=9Fe?= Cc: emacs-orgmode@gnu.org Hello, Sebastian Reu=C3=9Fe writes: > * testing/lisp/test-org.el: Add test. > --- Thank you. > testing/examples/refile/a.org | 6 ++++++ > + > +;;; org-refile Nitpick: Sections in test-org.el are sorted alphabetically. So the new "Refile" section could go between "Radio Targets" and "Sparse trees". > +(ert-deftest test-org/org-refile-get-targets () > + "Test `org-refile-get-targets'." > + (save-window-excursion > + (let ((examples-dir (file-truename "../examples/refile/"))) > + (cd examples-dir) > + (find-file-read-only "a.org") > + (find-file-read-only "b.org") > + (rename-buffer "gratuitous-prefix/b.org") > + (let ((org-refile-targets '((("a.org" "b.org") :level . 2))) > + (testcases > + `((nil . ("a/1/2" > + "a/2/2" > + "b/1/2" > + "b/2/2")) > + (file . ("a.org" > + "a.org/a\\/1\\/1/a\\/1\\/2" > + "a.org/a\\/2\\/1/a\\/2\\/2" > + "b.org" > + "b.org/b\\/1\\/1/b\\/1\\/2" > + "b.org/b\\/2\\/1/b\\/2\\/2")) > + (full-file-path . ,(mapcar (lambda (s) (concat examples-dir s)) > + '("a.org" > + "a.org/a\\/1\\/1/a\\/1\\/2" > + "a.org/a\\/2\\/1/a\\/2\\/2" > + "b.org" > + "b.org/b\\/1\\/1/b\\/1\\/2" > + "b.org/b\\/2\\/1/b\\/2\\/2"))) > + (buffer-name . ("a.org" > + "a.org/a\\/1\\/1/a\\/1\\/2" > + "a.org/a\\/2\\/1/a\\/2\\/2" > + "gratuitous-prefix/b.org" > + "gratuitous-prefix/b.org/b\\/1\\/1/b\\/1\\/2" > + "gratuitous-prefix/b.org/b\\/2\\/1/b\\/2\\/2"))))) > + (cl-loop for (use-outline-path . expected-targets) in testcases > + do (let ((org-refile-use-outline-path use-outline-path)) > + (should > + (equal > + (mapcar #'car > + (org-refile-get-targets)) > + expected-targets)))))))) > + Would it be possible to split this big test into smaller ones, with a description about what is really tested? See other tests in "test-org.el" for some examples. Big tests tend to not being very informative when they fail. IMO, code duplication is not an issue in test files when it makes tests more readable/useful. It would be even better if you can avoid relying on real files ("a.org" and "b.org" in your patch), but if it makes the test too convoluted, no worries. Regards, --=20 Nicolas Goaziou