Problem/Motivation

Currently, the module doesn't have any tests or schema files defined for the configurations. It would be great if some tests are added to ensure proper working of the module.

Steps to reproduce

NA

Proposed resolution

Write the following tests for the module:
1. Check website works when the module is installed.
2. Check if the admin user can configure the module settings.
3. Check if the email is actually blocked with the module enabled.
4. Check if the email message body gets logged in log messages.

Remaining tasks

Write all the tests.

User interface changes

None.

API changes

None. Although introducing of schema might require changes in configuration files.

Data model changes

None.

CommentFileSizeAuthor
#6 3263315-2.patch5.49 KBbinnythomas
#5 add-unit-tests-3263315.patch2.2 KBbinnythomas
#3 3263315.patch1.26 KBbinnythomas
#2 3263315.patch0 bytesbinnythomas

Comments

gaurav.kapoor created an issue. See original summary.

binnythomas’s picture

binnythomas commented
StatusFileSize
new 3263315.patch0 bytes

Adding patch with a single test.

binnythomas’s picture

binnythomas commented
StatusFileSize
new 3263315.patch1.26 KB

Ignore earlier patch. The current one has the tests.

gaurav.kapoor’s picture

gaurav.kapoor commented
Status: Active » Needs work
  1. Instead of this it would be better to add a schema for the module. 
          +  protected $strictConfigSchema = FALSE;
  2. 2 line spaces after class declaration can be changed to one.
binnythomas’s picture

binnythomas commented
Status: Needs work » Needs review
StatusFileSize
new add-unit-tests-3263315.patch2.2 KB

Updated the patch with the following changes. Please review and test.

  1. Added new schema file
  2. Removed schema override in unit tests
  3. Changed the name and structure of the configuration settings file.
binnythomas’s picture

binnythomas commented
StatusFileSize
new 3263315-2.patch5.49 KB

Adding a revised patch with the following changes.

1. Added a new automated PHP unit test for testing the configuration form.
2. Changes to the config variable naming to ensure consistency throughout the module with the name of the config variable in the schema file.

Regarding testing of mail sending and email blocking - As per the usual rule of writing tests we should test dependencies and sending of mails is a dependency. We might have to think of mocking sending of emails. Also, the key functionality is done via hook and as hooks are related to the API and not part of custom code they need not be tested. What do you think of this?

gaurav.kapoor’s picture

gaurav.kapoor commented

1. Minor CS issues when applying the patch

3263315-2.patch:34: new blank line at EOF.
+
warning: 1 line adds whitespace errors.

2. Class LoadTest can be renamed to EmailBlockerTest.

3. Typo in line 43 of EmailBlockerAdminConfigTest.php file.

   * Tests that the Email blcoke config page is accessible and fields exist.

4. Unwanted import statement in EmailBlockerAdminConfigTest.php file.

use Drupal\Core\Url;
gaurav.kapoor’s picture

gaurav.kapoor commented
Status: Needs review » Needs work
gaurav.kapoor’s picture

gaurav.kapoor commented

Tests similar to those in this module https://git.drupalcode.org/project/reroute_email/-/tree/2.1.x/tests/src/... can be written for the email blocker module as well. All those can be taken up in a different issue as well.