GNU bug report logs - #24313
[PATCH] Add tests for dom.el

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 24313 in the body.
You can then email your comments to 24313 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Simen Heggestøyl <simenheg <at> gmail.com>
To: larsi <at> gnus.org
Cc: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Add tests for dom.el
Date: Fri, 26 Aug 2016 20:26:10 +0200

[Message part 1 (text/plain, inline)]

Hello,

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

Do they look okay to add?

-- Simen

[0001-Add-tests-for-dom.el.patch (text/x-patch, attachment)]

Message #8 received at 24313 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Simen Heggestøyl <simenheg <at> gmail.com>
Cc: larsi <at> gnus.org, 24313 <at> debbugs.gnu.org
Subject: Re: bug#24313: [PATCH] Add tests for dom.el
Date: Sat, 27 Aug 2016 11:18:25 +0300

> Date: Fri, 26 Aug 2016 20:26:10 +0200
> From: Simen Heggestøyl <simenheg <at> gmail.com>
> Cc: 24313 <at> 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.

Message #11 received at 24313 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Simen Heggestøyl <simenheg <at> gmail.com>
Cc: larsi <at> gnus.org, 24313 <at> debbugs.gnu.org
Subject: Re: bug#24313: [PATCH] Add tests for dom.el
Date: Sat, 27 Aug 2016 12:07:13 +0200

Simen Heggestøyl <simenheg <at> 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.

Message #14 received at 24313 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Simen Heggestøyl <simenheg <at> gmail.com>
Cc: 24313 <at> debbugs.gnu.org
Subject: Re: bug#24313: [PATCH] Add tests for dom.el
Date: Sat, 27 Aug 2016 15:46:38 +0200

Simen Heggestøyl <simenheg <at> 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

Message #17 received at 24313 <at> debbugs.gnu.org (full text, mbox):

From: Simen Heggestøyl <simenheg <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>, Michael Albinus
 <michael.albinus <at> gmx.de>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 24313 <at> debbugs.gnu.org
Subject: Re: bug#24313: [PATCH] Add tests for dom.el
Date: Sun, 28 Aug 2016 10:58:47 +0200

On Sat, Aug 27, 2016 at 12:07 PM, Michael Albinus 
<michael.albinus <at> 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 <at> 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 <at> gnus.org> 
wrote:
> Looks good to me.

Good, I'll install it shortly.

Thank you all for the feedback.

-- Simen

Message #20 received at 24313 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Simen Heggestøyl <simenheg <at> gmail.com>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, Eli Zaretskii <eliz <at> gnu.org>,
 24313 <at> debbugs.gnu.org
Subject: Re: bug#24313: [PATCH] Add tests for dom.el
Date: Sun, 28 Aug 2016 11:21:30 +0200

Simen Heggestøyl <simenheg <at> 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.

Message #25 received at 24313-done <at> debbugs.gnu.org (full text, mbox):

From: Simen Heggestøyl <simenheg <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, Eli Zaretskii <eliz <at> gnu.org>,
 24313-done <at> debbugs.gnu.org
Subject: Re: bug#24313: [PATCH] Add tests for dom.el
Date: Sun, 28 Aug 2016 18:38:00 +0200

On Sun, Aug 28, 2016 at 11:21 AM, Michael Albinus 
<michael.albinus <at> 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

Message #28 received at 24313-done <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Simen Heggestøyl <simenheg <at> gmail.com>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, Eli Zaretskii <eliz <at> gnu.org>,
 24313-done <at> debbugs.gnu.org
Subject: Re: bug#24313: [PATCH] Add tests for dom.el
Date: Sun, 28 Aug 2016 19:41:24 +0200

Simen Heggestøyl <simenheg <at> 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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.