1) The unit test \Drupal\Tests\swiftmailer\Unit\Plugin\Mail\SwiftMailerTest doesn't really work because the code doesn't run well in isolation.

  • Have to redefine some constants from swiftmailer.module
  • Missing default config e.g. 'format' is blank which is not really a valid test
  • Missing dependencies, in particular the renderer is needed for massageMessageBody() to run.

In any case, there is an almost identical test Kernel test below. Let's just delete this one and move the testing it does into the Kernel test.

2) The kernel test \Drupal\Tests\swiftmailer\Kernel\Plugin\Mail\SwiftMailerTest has some decent tests but could be improved.

  • Test with both plain text and HTML formats.
  • Test with special characters (&, <) to prove that the conversions are working.
  • Better to test calling the public format() function rather than the internal massageMessageBody().

3) Add more tests

CommentFileSizeAuthor
#31 swiftmailer.tests_.3124110-31.patch3.97 KBadamps
#28 swiftmailer.tests_.3124110-28.patch5.86 KBadamps
#26 swiftmailer.tests_.3124110-26.patch3.5 KBadamps
#23 swiftmailer.tests_.3124110-23.patch2.83 KBadamps
#21 swiftmailer.tests_.3124110-21.patch2.33 KBadamps
#19 swiftmailer.tests_.3124110-19.patch1.54 KBadamps
#17 swiftmailer.tests_.3124110-17.patch928 bytesadamps
#15 swiftmailer.tests_.3124110-15.patch634 bytesadamps
#10 swiftmailer.tests_.3124110-10.patch4.7 KBadamps
#8 swiftmailer.tests_.3124110-8.patch652 bytesadamps
#5 swiftmailer.tests_.3124110-5.patch3.87 KBadamps

Comments

AdamPS created an issue. See original summary.

geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented
Related issues: +#2892104: Add support for CC and BCC headers

Can you look into #2892104: Add support for CC and BCC headers if the test mechanism there helps in any way? SHould we commit that as is or merge into any other?

adamps’s picture

adamps commented
Title: Fix/improve the tests » Fix/improve the tests for SwiftMailer::format

From first glance I don't think there is much in common. The other issue is about testing the mail() method whereas I am talking about testing the format() method. I updated the title to clarify that.

geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented

THx!

adamps’s picture

adamps commented
Status: Active » Needs review
StatusFileSize
new swiftmailer.tests_.3124110-5.patch3.87 KB

Patch for step 1: remove the unit test

  • AdamPS committed 8a57dac on 8.x-2.x 
    Issue #3124110 by AdamPS: Fix/improve the tests for SwiftMailer::format...
adamps’s picture

adamps commented
Status: Needs review » Active

I will commit each step separately so that it's possible to trace the history more easily in git.

adamps’s picture

adamps commented
StatusFileSize
new swiftmailer.tests_.3124110-8.patch652 bytes

Next step: rename the test to match what it does

  • AdamPS committed 7f53d7f on 8.x-2.x 
    Issue #3124110 by AdamPS: Fix/improve the tests for SwiftMailer::format...
adamps’s picture

adamps commented
StatusFileSize
new swiftmailer.tests_.3124110-10.patch4.7 KB

  • AdamPS committed 9c854d1 on 8.x-2.x 
    Issue #3124110 by AdamPS: Fix/improve the tests for SwiftMailer::format...
adamps’s picture

adamps commented

Patch for step 2. It's pretty amusing the things I had to write for expected values to make the tests run with the current code.

geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented

> I will commit each step separately so that it's possible to trace the history more easily in git.

I really like this approach.

adamps’s picture

adamps commented
Issue summary: View changes
adamps’s picture

adamps commented
StatusFileSize
new swiftmailer.tests_.3124110-15.patch634 bytes

Oops missed 1 call to massageMessageBody()

  • AdamPS committed 007681a on 8.x-2.x 
    Issue #3124110 by AdamPS: Fix/improve the tests for SwiftMailer::format
adamps’s picture

adamps commented
Status: Active » Needs review
StatusFileSize
new swiftmailer.tests_.3124110-17.patch928 bytes

Status: Needs review » Needs work

The last submitted patch, 17: swiftmailer.tests_.3124110-17.patch, failed testing. View results

adamps’s picture

adamps commented
Status: Needs work » Needs review
StatusFileSize
new swiftmailer.tests_.3124110-19.patch1.54 KB

Status: Needs review » Needs work

The last submitted patch, 19: swiftmailer.tests_.3124110-19.patch, failed testing. View results

adamps’s picture

adamps commented
Status: Needs work » Needs review
StatusFileSize
new swiftmailer.tests_.3124110-21.patch2.33 KB

Status: Needs review » Needs work

The last submitted patch, 21: swiftmailer.tests_.3124110-21.patch, failed testing. View results

adamps’s picture

adamps commented
Status: Needs work » Needs review
StatusFileSize
new swiftmailer.tests_.3124110-23.patch2.83 KB

  • AdamPS committed 820e8cf on 8.x-2.x 
    Issue #3124110 by AdamPS: Fix/improve the tests for SwiftMailer::format...
adamps’s picture

adamps commented
Status: Needs review » Fixed

Add inline CSS test. That seems pretty good for now, let's mark as fixed.

adamps’s picture

adamps commented
Status: Fixed » Needs review
StatusFileSize
new swiftmailer.tests_.3124110-26.patch3.5 KB

New patch to create SwiftMailerTestBase including use of AssertMailTrait to check mails that have been sent.

Status: Needs review » Needs work

The last submitted patch, 26: swiftmailer.tests_.3124110-26.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

adamps’s picture

adamps commented
Status: Needs work » Needs review
StatusFileSize
new swiftmailer.tests_.3124110-28.patch5.86 KB

  • AdamPS committed 6de5379 on 8.x-2.x 
    Issue #3124110 by AdamPS: Add SwiftMailerTestBase
adamps’s picture

adamps commented
Status: Needs review » Fixed
adamps’s picture

adamps commented
Status: Fixed » Needs review
StatusFileSize
new swiftmailer.tests_.3124110-31.patch3.97 KB

Some real mails test generated by Drupal Core. Mostly they prove that Core sucks with special characters in mails.

  • AdamPS committed 9c2858e on 8.x-2.x 
    Issue #3124110 by AdamPS: Fix/improve the tests for SwiftMailer::format...
adamps’s picture

adamps commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.