bug#24313: [PATCH] Add tests for dom.el

bug#24313: [PATCH] Add tests for dom.el

[Simen Heggestøyl]

[-- Attachment #1: Type: text/plain, Size: 103 bytes --]

Hello,

I saw that dom.el didn't have any tests, so I wrote some.

Do they look okay to add?

-- Simen

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-tests-for-dom.el.patch --]
[-- Type: text/x-patch, Size: 8066 bytes --]

From c74c0fac972ce235954efc56ff540021f9dd0615 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Simen=20Heggest=C3=B8yl?= <simenheg@gmail.com>
Date: Sun, 14 Aug 2016 16:03:50 +0200
Subject: [PATCH] Add tests for dom.el

* test/lisp/dom-tests.el: New file with tests for dom.el.
---
 test/lisp/dom-tests.el | 195 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 195 insertions(+)
 create mode 100644 test/lisp/dom-tests.el

diff --git a/test/lisp/dom-tests.el b/test/lisp/dom-tests.el
new file mode 100644
index 0000000..e3e14b2
--- /dev/null
+++ b/test/lisp/dom-tests.el
@@ -0,0 +1,195 @@
+;;; dom-tests.el --- Tests for dom.el  -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2016 Free Software Foundation, Inc.
+
+;; Author: Simen Heggestøyl <simenheg@gmail.com>
+;; Keywords:
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;;
+
+;;; Code:
+
+(require 'dom)
+(require 'ert)
+(require 'subr-x)
+
+(defun dom-test-tree ()
+  "Return a DOM tree for testing."
+  (dom-node "html" nil
+            (dom-node "head" nil
+                      (dom-node "title" nil
+                                "Test"))
+            (dom-node "body" nil
+                      (dom-node "div" '((class . "foo")
+                                        (style . "color: red;"))
+                                (dom-node "p" '((id . "bar"))
+                                          "foo"))
+                      (dom-node "div" '((title . "2nd div"))
+                                "bar"))))
+
+(ert-deftest test-dom-tag ()
+  (let ((dom (dom-test-tree)))
+    (should (equal (dom-tag dom) "html"))
+    (should (equal (dom-tag (car (dom-children dom))) "head"))))
+
+(ert-deftest test-dom-attributes ()
+  (let ((dom (dom-test-tree)))
+    (should (null (dom-attributes dom)))
+    (should (equal (dom-attributes (dom-by-class dom "foo"))
+                   '((class . "foo") (style . "color: red;"))))))
+
+(ert-deftest test-dom-children ()
+  (let ((dom (dom-test-tree)))
+    (should (equal (mapcar #'dom-tag (dom-children dom))
+                   '("head" "body")))
+    (should (equal (dom-tag (dom-children (dom-children dom)))
+                   "title"))))
+
+(ert-deftest test-dom-non-text-children ()
+  (let ((dom (dom-test-tree)))
+    (should (equal (dom-children dom) (dom-non-text-children dom)))
+    (should (null (dom-non-text-children
+                   (dom-children (dom-children dom)))))))
+
+(ert-deftest test-dom-set-attributes ()
+  (let ((dom (dom-test-tree))
+        (attributes '((xmlns "http://www.w3.org/1999/xhtml"))))
+    (should (null (dom-attributes dom)))
+    (dom-set-attributes dom attributes)
+    (should (equal (dom-attributes dom) attributes))))
+
+(ert-deftest test-dom-set-attribute ()
+  (let ((dom (dom-test-tree))
+        (attr 'xmlns)
+        (value "http://www.w3.org/1999/xhtml"))
+    (should (null (dom-attributes dom)))
+    (dom-set-attribute dom attr value)
+    (should (equal (dom-attr dom attr) value))))
+
+(ert-deftest test-dom-attr ()
+  (let ((dom (dom-test-tree)))
+    (should (null (dom-attr dom 'id)))
+    (should (equal (dom-attr (dom-by-id dom "bar") 'id) "bar"))))
+
+(ert-deftest test-dom-text ()
+  (let ((dom (dom-test-tree)))
+    (should (string-empty-p (dom-text dom)))
+    (should (equal (dom-text (dom-by-tag dom "title")) "Test"))))
+
+(ert-deftest test-dom-texts ()
+  (let ((dom (dom-test-tree)))
+    (should (equal (dom-texts dom) "Test foo bar"))
+    (should (equal (dom-texts dom ", ") "Test, foo, bar"))))
+
+(ert-deftest test-dom-child-by-tag ()
+  (let ((dom (dom-test-tree)))
+    (should (equal (dom-child-by-tag dom "head")
+                   (car (dom-children dom))))
+    (should (not (dom-child-by-tag dom "title")))))
+
+(ert-deftest test-dom-by-tag ()
+  (let ((dom (dom-test-tree)))
+    (should (= (length (dom-by-tag dom "div")) 2))
+    (should (null (dom-by-tag dom "article")))))
+
+(ert-deftest test-dom-strings ()
+  (let ((dom (dom-test-tree)))
+    (should (equal (dom-strings dom) '("Test" "foo" "bar")))
+    (should (equal (dom-strings (dom-children dom)) '("Test")))))
+
+(ert-deftest test-dom-by-class ()
+  (let ((dom (dom-test-tree)))
+    (should (equal (dom-tag (dom-by-class dom "foo")) "div"))
+    (should (null (dom-by-class dom "bar")))))
+
+(ert-deftest test-dom-by-style ()
+  (let ((dom (dom-test-tree)))
+    (should (equal (dom-tag (dom-by-style dom "color")) "div"))
+    (should (null (dom-by-style dom "width")))))
+
+(ert-deftest test-dom-by-id ()
+  (let ((dom (dom-test-tree)))
+    (should (equal (dom-tag (dom-by-id dom "bar")) "p"))
+    (should (null (dom-by-id dom "foo")))))
+
+(ert-deftest test-dom-elements ()
+  (let ((dom (dom-test-tree)))
+    (should (equal (dom-elements dom 'class "foo")
+                   (dom-by-class dom "foo")))
+    (should (equal (dom-attr (dom-elements dom 'title "2nd") 'title)
+                   "2nd div"))))
+
+(ert-deftest test-dom-remove-node ()
+  (let ((dom (dom-test-tree)))
+    (should (not (dom-remove-node dom dom)))
+    (should (= (length (dom-children dom)) 2))
+    (dom-remove-node dom (car (dom-children dom)))
+    (should (= (length (dom-children dom)) 1))
+    (dom-remove-node dom (car (dom-children dom)))
+    (should (null (dom-children dom)))))
+
+(ert-deftest test-dom-parent ()
+  (let ((dom (dom-test-tree)))
+    (should (not (dom-parent dom dom)))
+    (should (equal (dom-parent dom (car (dom-children dom))) dom))))
+
+(ert-deftest test-dom-previous-sibling ()
+  (let ((dom (dom-test-tree)))
+    (should (not (dom-previous-sibling dom dom)))
+    (let ((children (dom-children dom)))
+      (should (equal (dom-previous-sibling dom (cadr children))
+                     (car children))))))
+
+(ert-deftest test-dom-append-child ()
+  (let ((dom (dom-test-tree)))
+    (should (equal (mapcar #'dom-tag (dom-children dom))
+                   '("head" "body")))
+    (dom-append-child dom (dom-node "feet"))
+    (should (equal (mapcar #'dom-tag (dom-children dom))
+                   '("head" "body" "feet")))))
+
+(ert-deftest test-dom-add-child-before ()
+  (let ((dom (dom-test-tree)))
+    (should (equal (mapcar #'dom-tag (dom-children dom))
+                   '("head" "body")))
+    (dom-add-child-before dom (dom-node "neck")
+                          (dom-child-by-tag dom "body"))
+    (should (equal (mapcar #'dom-tag (dom-children dom))
+                   '("head" "neck" "body")))
+    (dom-add-child-before dom (dom-node "hat"))
+    (should (equal (mapcar #'dom-tag (dom-children dom))
+                   '("hat" "head" "neck" "body")))))
+
+(ert-deftest test-dom-ensure-node ()
+  (let ((node (dom-node "foo")))
+    (should (equal (dom-ensure-node '("foo")) node))
+    (should (equal (dom-ensure-node '(("foo"))) node))
+    (should (equal (dom-ensure-node '("foo" nil)) node))
+    (should (equal (dom-ensure-node '(("foo") nil)) node))))
+
+(ert-deftest test-dom-pp ()
+  (let ((node (dom-node "foo" nil "")))
+    (with-temp-buffer
+      (dom-pp node)
+      (should (equal (buffer-string) "(\"foo\" nil\n \"\")")))
+    (with-temp-buffer
+      (dom-pp node t)
+      (should (equal (buffer-string) "(\"foo\" nil)")))))
+
+(provide 'dom-tests)
+;;; dom-tests.el ends here
-- 
2.9.3

bug#24313: [PATCH] Add tests for dom.el

[Eli Zaretskii]

> Date: Fri, 26 Aug 2016 20:26:10 +0200
> From: Simen Heggestøyl <simenheg@gmail.com>
> Cc: 24313@debbugs.gnu.org
> 
> I saw that dom.el didn't have any tests, so I wrote some.
> 
> Do they look okay to add?

Tests are always okay to add and are very welcome, so thank you very
much for this contribution.

If there are specific aspects of these tests about which you are
unsure or which you'd like us to consider, please point them out.
Otherwise, if no comments are posted, let's push this in a few days.

bug#24313: [PATCH] Add tests for dom.el

[Michael Albinus]

Simen Heggestøyl <simenheg@gmail.com> writes:

> Hello,

Hi Simen,

> I saw that dom.el didn't have any tests, so I wrote some.
>
> Do they look okay to add?
>
> +;;; dom-tests.el --- Tests for dom.el  -*- lexical-binding: t; -*-
> +
> +(ert-deftest test-dom-tag ()

Pls start all tests with the name of the package, "dom-tests-*". It
would also be nice if you could add a docstring per test.

> +    (should (null (dom-attributes dom)))

This reads better as

(should-not (dom-attributes dom))

Best regards, Michael.

bug#24313: [PATCH] Add tests for dom.el

[Lars Ingebrigtsen]

Simen Heggestøyl <simenheg@gmail.com> writes:

> I saw that dom.el didn't have any tests, so I wrote some.
>
> Do they look okay to add?

Looks good to me.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#24313: [PATCH] Add tests for dom.el

[Simen Heggestøyl]

On Sat, Aug 27, 2016 at 12:07 PM, Michael Albinus 
<michael.albinus@gmx.de> wrote:
> Pls start all tests with the name of the package, "dom-tests-*".

OK, I'll change the test names.

> It would also be nice if you could add a docstring per test.

Does it any value to such trivial test cases? For most of these I don't
think there's more to add than "Tests <name-of-function>.", which is
already conveyed by the name of the test.

> This reads better as
> 
> (should-not (dom-attributes dom))

OK, I'll change it.

On Sat, Aug 27, 2016 at 10:18 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> If there are specific aspects of these tests about which you are
> unsure or which you'd like us to consider, please point them out.

Nothing in particular, I just wanted to get feedback before installing.

On Sat, Aug 27, 2016 at 3:46 PM, Lars Ingebrigtsen <larsi@gnus.org> 
wrote:
> Looks good to me.

Good, I'll install it shortly.

Thank you all for the feedback.

-- Simen

bug#24313: [PATCH] Add tests for dom.el

[Michael Albinus]

Simen Heggestøyl <simenheg@gmail.com> writes:

Hi Simen,

>> It would also be nice if you could add a docstring per test.
>
> Does it any value to such trivial test cases? For most of these I don't
> think there's more to add than "Tests <name-of-function>.", which is
> already conveyed by the name of the test.

It depends. Likely you are right, but sometimes it might be worth to
note also what is NOT covered by a test, or to mention possible side
effects which are tested implicitely. But it is your decision, of course.

> Thank you all for the feedback.
>
> -- Simen

Best regards, Michael.

bug#24313: [PATCH] Add tests for dom.el

[Simen Heggestøyl]

On Sun, Aug 28, 2016 at 11:21 AM, Michael Albinus 
<michael.albinus@gmx.de> wrote:
> It depends. Likely you are right, but sometimes it might be worth to
> note also what is NOT covered by a test, or to mention possible side
> effects which are tested implicitely. But it is your decision, of 
> course.

I agree with you. I went over the tests once more with this in mind and
found one instance which I think was worth a comment, but still think
the rest are trivial enough to stand on their own.

I've installed the tests with the changes you suggested last time.

Thanks again for your time.

-- Simen

bug#24313: [PATCH] Add tests for dom.el

[Michael Albinus]

Simen Heggestøyl <simenheg@gmail.com> writes:

> I've installed the tests with the changes you suggested last time.

Thanks!

> Thanks again for your time.
>
> -- Simen

Best regards, Michael.