Write mail display tests per example

I guess it will be an own test class.

And yes, we need:
- specific tests of individual parts, one per part, Unit test.
- multiple tests that each process an example message and assert expectations.

So we discussed how to display mails in #2804587: Create mail display mock-ups

We won't need an element per part type.
Instead the message will be an intelligent element containing 4 parts: Header, Body, Attachments, Unknown.

The tests will focus on those areas.
One test for the headers will process multiple variations of from / to / subject messages to make sure single / multiple recipients or such with or without name are well displayed. Also expansion of header area is possible.
Some other test will process multiple mails: a plaintext, one with alternate (plaintext AND html), one with HTML only. And the body needs to display it properly.
Some other test will focus on multiple attachment variants. With short, long labels, containing special characters, cover case of attachment deletion due to oversize, and inline image as attachments
One test will introduce unknown parts that are not considered with the elements mentioned above. They need to get visible. The regular cases need to check that the unknown area is not visible.
Some later test will focus on inline image coverage (not yet supported).

After started with the basics, we should move the test coverage expectations into the corresponding issues. Until this happened, the issue will stay back open.

There have been some unexpected CI errors, with re-adding test seems everything is ok. Setting back to needs review :)

1. +++ b/tests/modules/inmail_test/eml/multiple-header-fields.eml
   @@ -0,0 +1,32 @@
   +Cc: Someone Else <inmail_other@example.com>From: Nancy <nancy@example.com>
   

   Uh?

2. +++ b/tests/modules/inmail_test/eml/multiple-header-fields.eml
   @@ -0,0 +1,32 @@
   +Bcc: Big Brother <bigbrother@example.com>
   

   We as recipient will never see a mail with Bcc in header. The MTA is expanding Bcc recipient sand by definition remove the header from the mail before sending it out.

3. +++ b/tests/modules/inmail_test/eml/multiple-header-fields.eml
   @@ -0,0 +1,32 @@
   +Content-Type: text/plain; charset=UTF-8
   +To: bounces+user=example.org@example.com
   +Cc: Senator John <senator_john@usa.gov.com>
   +From: hello_world@example.com
   +Subject: I am currently working
   +Date: Wed, 8 Jan 2014 13:42:55 +0100
   ...
   +--047d7bf0d0f694271f0505eb5984
   +Content-Type: text/html; charset=UTF-8
   +Date: Wed, 08 Jan 2014 13:42:18 +0100
   +From: Arild Matsson <admin@example.com>
   +To: replica@example.com
   +Subject: Replica
   

   Parts in multipart mails like this (especially type text/plain or text/html) are never mails and thus never have a From / To / Cc header.

   Only the mail header itself needs those fields. The mockup shows that we will remove that "From / To" display per part and display parts completely different.

Removed wrong headers.

Now the problem is:
- See #3, if we split, it should be a separate test class, not only a test method.
- The existing mail display tests also need to be moved here.

+++ b/tests/modules/inmail_test/eml/multiple-header-fields.eml
@@ -0,0 +1,26 @@
+From: Arild Matsson <arild@masked1.se>
...
+From: Nancy <nancy@example.com>

This is invalid.

+++ b/src/Tests/IntegrationTest.php
@@ -189,6 +189,31 @@ class IntegrationTest extends WebTestBase {
+    $this->assertText('Email display');

Most importantly also assert the header fields. Or negatively assert them if they should be missing.

And it should be multiple example mails, with variable cases. And most importantly we need to cover varying cases with multiple recipient (to, cc) addresses.

Made new test class and move relevant functions there. Also added new helper method. I need to change xpath in helper function and check the order of fields.

I don't know why but git says that I renamed the old class IntegrationTest instead of creating new MultipartMessageHeaderTest.

+++ b/src/Tests/MultipartMessageHeaderTest.php
@@ -91,14 +91,7 @@ class IntegrationTest extends WebTestBase {
+    $this->assertHeaderFields($message);

@@ -135,130 +127,44 @@ class IntegrationTest extends WebTestBase {
+ public function assertHeaderFields($message) {

That's too much abstracted away.
In maximum we can offer a assertHeaderField($name, $value) and assertNoHeaderField($name) and call them based on our expectations.

+++ b/src/Tests/IntegrationTest.php
similarity index 53%
copy from src/Tests/IntegrationTest.php

copy from src/Tests/IntegrationTest.php
copy to src/Tests/MultipartMessageHeaderTest.php

No it says you started with a copy and then only indicates changes to it. That makes patches smaller and easier to review.

Added three new function, two as @miro suggested and one more to check if the value is on right field.

Wrong patch.

Small thing:

+++ b/src/Tests/MultipartMessageHeaderTest.php
@@ -0,0 +1,225 @@
+    $event_id = db_query_range('SELECT event_id FROM {past_event} WHERE machine_name = :machine_name ORDER BY event_id DESC', 0, 1, array(':machine_name' => $machine_name))->fetchField();

[] instead of array.

Changed to use new syntax for array.

1. +++ b/src/Tests/MultipartMessageHeaderTest.php
   @@ -0,0 +1,225 @@
   +use Drupal\search_api\Tests\WebTestBase;
   

   search_api?!

2. +++ b/src/Tests/MultipartMessageHeaderTest.php
   @@ -0,0 +1,225 @@
   +    // Test "teaser" view mode of Inmail message element.
   ...
   +    $user = $this->drupalCreateUser([
   +      'access administration pages',
   +      'administer inmail',
   +      ]);
   +    $this->drupalLogin($user);
   

   Should go into setup.

3. +++ b/src/Tests/MultipartMessageHeaderTest.php
   @@ -0,0 +1,225 @@
   +    $regular = drupal_get_path('module', 'inmail_test') . '/eml/multiple-header-fields.eml';
   +    $raw_multipart = file_get_contents(DRUPAL_ROOT . '/' . $regular);
   ...
   +    $processor = \Drupal::service('inmail.processor');
   +    \Drupal::state()->set('inmail.test.success', '');
   +    // Process the raw multipart mail message.
   +    $processor->process('unique_key', $raw_multipart, DelivererConfig::create(['id' => 'test']));
   

   I think we should offer a $this->processMessage($filename).

4. +++ b/src/Tests/MultipartMessageHeaderTest.php
   @@ -0,0 +1,225 @@
   +    $this->checkOrderOfFields('Received', 'inmail-message__element inmail-message__header__received-date');
   ...
   +  public function checkOrderOfFields($name, $field_class) {
   

   How is this an "Order"?
   Anyway, it's an assert, so the method should start with that.

   This kind of assert is not too common.

5. +++ b/src/Tests/MultipartMessageHeaderTest.php
   @@ -0,0 +1,225 @@
   +  public function assertNoHeaderField($name) {
   +    $this->assertNoText($name);
   ...
   +  public function assertHeaderField($name, $value) {
   +    $this->assertText($name);
   +    $this->assertText($value);
   

   This is highly ambiguous.
   This is where you want to use xpath.

6. +++ b/tests/modules/inmail_test/eml/multiple-header-fields.eml
   @@ -0,0 +1,25 @@
   +Subject: I am currently working
   ...
   +Subject: Replica
   

   This header is unexpected for a text/html or text/plain part type.

Finally I finished the tests and fixed test fails. Now tests doesn't depending of parseAddress issue. Now test pass and should pass when we change parseAdress.

+++ b/src/Tests/MultipartMessageHeaderTest.php
@@ -0,0 +1,204 @@
+    return $this->assertTrue($exist);
...
+    return $this->assertTrue($exist);

In case of a fail, we need to pass in some meaningful message.

You should/could move the assert helper for that mail Header into a trait. Then it's easy to reuse it in different tests too.

I created trait AssertHeaderFieldTrait and some function for testing. I creted our function assertContains() because I discussed with @Berdit and we can not use the one witch exist. Patch is not ready yet, just uploading patch to check if I am in right way.

I will look into this again.

I've rerolled and refactored the previous patch since I guess @miro_dietiker doesn't have too much time to fix this.
No interdiff, since it is bigger.
Made some progress providing the assertHelper() methods, but still needs some improvements (like assertElementHeaderField(), "to cover the received-date value" as well). Otherwise it worked pretty fine locally. Didn't tested if these changes affects the other tests though, but we will see if testbot will complain.

Remaining tasks:
- check the @todo's
- and evtl. split the tests that I've marked (// Test 1: ..., // Test 2: ..., ...) into separate test methods

Leave back to whoever wants to work on this. Tomorrow I'll need to work on another project.

Ok, after more discussion I've provided more asserts methods trying to make it more "loose coupling" as possible:

• for general address header field, assertAddressHeaderField
• for address display name, assertAddressDisplayName()
• for email address, assertEmailAddress()
• for general header field element (like Date or Subject), assertElementHeaderField()

and started to write an assertUnsubscribeHeaderField() and getHeaderFieldUnsubscribeXpath() but not sure if it is necessary.

Put some @todos, some of them need to be checked now, the other ones can be fixed later as followups (writing the missing tests).

Sorry for taking too long for this "refactoring" issue.

Further discussions, reroll and fixing filenames that have been changed during the week.
Currently providing the conversation case as mentioned in #2828497: Create conversation tree mail (see comment #3). Work in progress.

The conversation mail and pseudo-test is clearly a separate issue and hasn't even an overlapping part.

There are private functions in here that could easily be protected.

Reverted that conversation part. Fixed InmailIntegrationTest.

Great work. Committed.

Please create the follow-ups from the @todo mentions.

Followups from verbal discussions:

• #2830190: Reduce time creating new test environment for "testbot performance"
• #2830186: Inconsistent Inmail HTML template div's classes using "-" and "_"
• #2830225: Separate "plain/html body message" tests from "attachment" tests

Followups from @todos:

• #2830207: Provide assert helpers for body message
• #2830222: Cleanup showcase test doTestSimpleEmailDisplay() and add explicit multipart alternative body message test
• #2830204: Move XSS part into separate email example

Still need to (open an issue?) discuss about a couple of @todos:

• rename again normal-forwarded.eml to simple-message.eml?
• use assertUnsubscribeHeaderField()/assertNoUnsubscribeHeaderField()?