Message body doubles up

This really seems to be wrong to me.
However let's see why the tests didn't show this issue.

Because there is no test assertion to check if something is shown twice in the mail body :) Tests' don't magically discover bugs, they only check what you could think of while writing them. I also haven't noticed this while printing the mails to write test assertions for them. So maybe this only occurs in a configuration that don't have any test for yet. (You have to keep in mind that our test suite is still very basic, there are lots of combinations which aren't tested yet, including having any other fields that the default body one on any newsletter node).

That's how tests evolve, you write a few, someone reports a bug which is not covered yet, you add another test case or just a simple assertion to check this as well and you can then be pretty certain that this regression won't happen again (only pretty certain because the test can have bugs too).

Also, I've never touched that function as you can see from the blame log, so I also never wrote any tests for it:

55fe6805 (Erik Stielstra       2010-02-24 12:36:50 +0000 2719)
55fe6805 (Erik Stielstra       2010-02-24 12:36:50 +0000 2720)       // Render the top-level DIV.
55fe6805 (Erik Stielstra       2010-02-24 12:36:50 +0000 2721)       $output .= '<div class="clearfix">' . $output . '</div>';

(in fact, the whole function is from the same commit back in 2010-02-24).

This leads to the conclusion that, while this line is clearly wrong, the occurence of this bug must have been triggered by something else.

Will take a closer look in the next days.

:-) Adding tag for test candidate. ;-)

Note that the common tag for this is "needs tests". "test candidate" is something that I used specifically when preparing for our testing workshop.

I just adopted your pattern. Feel free to update the issues. Also there was a typo in the original version and i needed around 5 issues to finally realize the tagname is wrong. ;-)
http://drupal.org/project/issues/search/simplenews?issue_tags=test%20can...

subscribe

Ok, attaching a patch with the fix including tests.

We didn't have any tests for sending HTML mails from actual nodes yet, only a basic one with a custom source implementation. That's why this bug wasn't found. I am not sure why it didn't happen with alpha1 though.

Also fixed another bug in the format override handling and explicitly pass the format to the footer template instead of getting in the preprocess function from the category to allow to override it.

Also added a custom MailSystem implementation used for testing that doesn't convert HTML to text.

Looks pretty fine. Great job, thank you. (Reviewed by reading code.)

Ok, commited!

I get the following when applying the patch:

root@UbuntuServer:/var/www/sites/all/modules/simplenews# git apply -v fix_doubled_content_with_tests.patch
Checking patch includes/simplenews.mail.inc...
Checking patch includes/simplenews.source.inc...
Checking patch simplenews.module...
error: while searching for:

    $edit = array(
      'title' => $this->randomName(),
      'body[und][0][value]' => "Mail token: [simplenews-subscriber:mail]",
    );
    $this->drupalPost('node/add/simplenews', $edit, ('Save'));
    $this->assertTrue(preg_match('|node/(\d+)$|', $this->getUrl(), $matches), 'Node created');

error: patch failed: simplenews.module:1796
error: simplenews.module: patch does not apply

any clue? thanks!

You don't need to apply this patch.

Just download the dev snapshot (you can switch back to the beta version once released) and you should be good to go.