tests: improve `_send_email` #3105

georglauterbach · 2023-02-22T10:23:48Z

Description

The function now handles IP, Port and more path names, as well as extra options you can supply to nc.

Type of change

Improvement (non-breaking change that does improve existing functionality)

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation (README.md or the documentation under docs/)
If necessary I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

The function now handles IP, Port and more path names, as well as extra options you can supply to `nc`.

test/tests/parallel/set1/spam_virus/fail2ban.bats

test/tests/parallel/set3/mta/smtp_delivery.bats

test/helper/sending.bash

Now, the method (invocation) really has become easier.

georglauterbach · 2023-02-22T11:17:55Z

This PR is waiting for #3006 :) (to avoid forcing the OP to resolve merge conflicts again).

polarathene

Sorry for the hassle, but I think _send_email() needs to swap param 2 and 3 around so the empty '' values can be dropped? 🤔

test/tests/parallel/set3/mta/smtp_delivery.bats

CONTAINER_NAME is now assumed to be set, which makes invocation even easier for common cases. Moreover, the helper that additionally retrieves the mail ID was renamed (_send_mail_* -> _send_email_*) to be canonical with _send_mail; it also takes the same arguments now, and has the same assumptions when calling it.

georglauterbach · 2023-02-22T14:54:26Z

CI really is not feeling well today. I'm seeing the same weird test failures as in #3006, even though both PRs do not touch Amavis at all. @polarathene you mentioned a timing issue, because our tests are now running with less delay. Should we introduce additional short sleep periods?

polarathene · 2023-02-23T00:28:02Z

Should we introduce additional short sleep periods?

Yeah, seems that may be necessary for this test. I think sleep 5 should be more than enough here?

Justification

docker-mailserver/test/tests/parallel/set3/mta/smtp_delivery.bats

Lines 50 to 59 in 9724060

    
           _run_in_container setup email add 'added@localhost.localdomain' 'mypassword' 
        
           assert_success 
        
           _wait_until_change_detection_event_completes 
        
           # Even if the Amavis port is reachable at this point, it may still refuse connections? 
        
           _wait_for_tcp_port_in_container 10024 
        
           _wait_for_smtp_port_in_container_to_respond 
        
           # Amavis may still not be ready to receive mail, sleep a little to avoid connection failures: 
        
           sleep 1

It's likely that the change detection completes, and reloads Amavis:

docker-mailserver/target/scripts/check-for-changes.sh

Lines 86 to 94 in 9724060

    
           function _reload_amavis 
        
           { 
        
             if [[ ${CHANGED} =~ ${DMS_DIR}/postfix-accounts.cf ]] || [[ ${CHANGED} =~ ${DMS_DIR}/postfix-virtual.cf ]] 
        
             then 
        
               # /etc/postfix/vhost was updated, amavis must refresh it's config by 
        
               # reading this file again in case of new domains, otherwise they will be ignored. 
        
               amavisd-new reload 
        
             fi 
        
           }

And that possibly isn't a quick process for Amavis to reload / restart, but ports may already be available 🤷‍♂️ bumping up the sleep should help. Optimizations that have been made was to load Postfix without a wrapper (and switch to not using chroot), which would have been 3-5 seconds faster IIRC, and the change detection no longer has a 10 second start-up delay (this PR adjusted the failing test in an attempt to compensate), both would have given Amavis plenty of time originally.

Common failure for future reference as action logs will eventually expire

not ok 30 [SMTP] (delivery) should succeed at emptying mail queue in 374ms
# (from function `refute_output' in file test/test_helper/bats-assert/src/refute_output.bash, line 189,
#  in test file test/tests/parallel/set3/mta/smtp_delivery.bats, line 99)
#   `refute_output --partial 'Connection refused'' failed
#
# -- output should not contain substring --
# substring (1 lines):
#   Connection refused
# output (50 lines):
#   -Queue ID-  --Size-- ----Arrival Time---- -Sender/Recipient-------
#   9E57A23993E*     493 Wed Feb 22 14:39:50  user@external.tld
#                                            user2@otherdomain.tld
#
#   AAA7C23997F*     482 Wed Feb 22 14:39:51  user@external.tld
#                                            user1@localhost.localdomain
#
#   2B81C2399[37](https://github.com/docker-mailserver/docker-mailserver/actions/runs/4243784120/jobs/7377011909#step:6:38)*     572 Wed Feb 22 14:39:50  spam@external.tld
#                                            user1@localhost.localdomain
#
#   1DEC62[39](https://github.com/docker-mailserver/docker-mailserver/actions/runs/4243784120/jobs/7377011909#step:6:40)97A*     505 Wed Feb 22 14:39:51  user@external.tld
#                                            added@localhost.localdomain
#
#   65D6123997E*     580 Wed Feb 22 14:39:51  user@external.tld
#                                            user1@localhost.localdomain
#
#   E2A6F239986*     490 Wed Feb 22 14:39:51  user@external.tld
#                                            user2@otherdomain.tld
#
#   D1C87239960*     505 Wed Feb 22 14:39:50  user@external.tld
#                                            user3@localhost.localdomain
#
#   6855C23993A*     505 Wed Feb 22 14:39:50  user@external.tld
#                                            user1@localhost.localdomain
#
#   E3AB2239932     514 Wed Feb 22 14:39:49  user@external.tld
#                      (connect to 127.0.0.1[127.0.0.1]:10024: Connection refused)
#                                            external1@otherdomain.tld
#
#   2B653239924     522 Wed Feb 22 14:39:49  user@external.tld
#                      (connect to 127.0.0.1[127.0.0.1]:10024: Connection refused)
#                                            user1@localhost.localdomain
#
#   A125B2398AF     553 Wed Feb 22 14:39:48  user@external.tld
#                      (connect to 127.0.0.1[127.0.0.1]:10024: Connection refused)
#                                            user1~test@localhost.localdomain
#
#   DDC6[42](https://github.com/docker-mailserver/docker-mailserver/actions/runs/4243784120/jobs/7377011909#step:6:43)398EA     510 Wed Feb 22 14:39:48  user@external.tld
#                      (connect to 127.0.0.1[127.0.0.1]:10024: Connection refused)
#                                            user1@localhost.localdomain
#
#   67C212398A5     516 Wed Feb 22 14:39:[48](https://github.com/docker-mailserver/docker-mailserver/actions/runs/4243784120/jobs/7377011909#step:6:49)  user@external.tld
#                      (connect to 127.0.0.1[127.0.0.1]:10024: Connection refused)
#                                            user1@localhost.localdomain
#
#   9ADFC23992E     537 Wed Feb 22 14:39:[49](https://github.com/docker-mailserver/docker-mailserver/actions/runs/4243784120/jobs/7377011909#step:6:50)  user@external.tld
#                      (connect to 127.0.0.1[127.0.0.1]:10024: Connection refused)
#                                            external1@otherdomain.tld
#
#   -- 7 Kbytes in 14 Requests.
# --

test/tests/serial/tests.bats

georglauterbach added 2 commits February 22, 2023 11:21

make _send_email more generic (& more powerful)

a47aeb2

The function now handles IP, Port and more path names, as well as extra options you can supply to `nc`.

adjust more code to use _send_email

bf72fcd

georglauterbach added area/scripts area/tests kind/improvement Improve an existing feature, configuration file or the documentation labels Feb 22, 2023

georglauterbach added this to the v12.0.0 milestone Feb 22, 2023

georglauterbach requested review from casperklein and polarathene February 22, 2023 10:23

georglauterbach self-assigned this Feb 22, 2023

polarathene requested changes Feb 22, 2023

View reviewed changes

test/tests/parallel/set1/spam_virus/fail2ban.bats Outdated Show resolved Hide resolved

test/tests/parallel/set3/mta/smtp_delivery.bats Outdated Show resolved Hide resolved

test/helper/sending.bash Outdated Show resolved Hide resolved

revise initial refactoring

1397786

Now, the method (invocation) really has become easier.

georglauterbach added the pr/waiting for other PR to get merged This PR is waiting for another / other PR(s) to get merged label Feb 22, 2023

georglauterbach force-pushed the improve/send-email branch from 119aa80 to 1397786 Compare February 22, 2023 11:23

polarathene requested changes Feb 22, 2023

View reviewed changes

test/tests/parallel/set3/mta/smtp_delivery.bats Outdated Show resolved Hide resolved

georglauterbach mentioned this pull request Feb 22, 2023

config: disable SMTP authentication on port 25 #3006

Merged

6 tasks

georglauterbach added 2 commits February 22, 2023 15:34

corrected Amavis test teardown file

83dbfbc

Update smtp_delivery.bats

02560a7

georglauterbach requested a review from polarathene February 23, 2023 08:19

polarathene previously approved these changes Feb 23, 2023

View reviewed changes

georglauterbach added 2 commits February 23, 2023 20:56

Merge remote-tracking branch 'origin/master' into improve/send-email

8f1a02c

adjust more manual send lines

f536218

georglauterbach dismissed polarathene’s stale review via f536218 February 23, 2023 20:12

georglauterbach requested a review from polarathene February 23, 2023 21:14

polarathene requested changes Feb 23, 2023

View reviewed changes

test/tests/serial/tests.bats Outdated Show resolved Hide resolved

chore: Fix typo

6db019f

polarathene approved these changes Feb 23, 2023

View reviewed changes

georglauterbach merged commit ae05e6a into master Feb 24, 2023

georglauterbach deleted the improve/send-email branch February 24, 2023 09:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests: improve `_send_email` #3105

tests: improve `_send_email` #3105

georglauterbach commented Feb 22, 2023

georglauterbach commented Feb 22, 2023 •

edited

polarathene left a comment

georglauterbach commented Feb 22, 2023

polarathene commented Feb 23, 2023

tests: improve _send_email #3105

tests: improve _send_email #3105

Conversation

georglauterbach commented Feb 22, 2023

Description

Type of change

Checklist:

georglauterbach commented Feb 22, 2023 • edited

polarathene left a comment

Choose a reason for hiding this comment

georglauterbach commented Feb 22, 2023

polarathene commented Feb 23, 2023

tests: improve `_send_email` #3105

tests: improve `_send_email` #3105

georglauterbach commented Feb 22, 2023 •

edited