Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests(fix): lmtp_ip.bats improve partial failure output #3552

Merged
merged 2 commits into from Sep 28, 2023

Conversation

polarathene
Copy link
Member

@polarathene polarathene commented Sep 28, 2023

Description

Instead of exit status of 124 (signifies timeout), it should fail with 1 (failure) like the others. Handled via using _run_in_container_bash() (timeout failure 124 does not propagate, and is treated as 1 instead).

In this case we are waiting on the status of the mail being sent, the pattern provided to grep is too specific and results in a timeout. Instead since we only expect the one log entry, match any status and assert the expected pattern afterwards.

This provides a more helpful failure output that informs us that mail was at least processed by Postfix, but the sent status is not what we expected.

Before

 ✗ [ENV] (POSTFIX_DAGENT) delivers mail to existing account [60327]
   (from function `assert_success' in file test/test_helper/bats-assert/src/assert_success.bash, line 42,
    in test file test/tests/parallel/set3/mta/lmtp_ip.bats, line 47)
     `assert_success' failed

   -- command failed --
   status : 124
   output :
   --

After

 ✗ [ENV] (POSTFIX_DAGENT) delivers mail to existing account [1425]
   (from function `assert_output' in file test/test_helper/bats-assert/src/assert_output.bash, line 178,
    in test file test/tests/parallel/set3/mta/lmtp_ip.bats, line 48)
     `assert_output --regexp "${MATCH_LOG_LINE}=sent .* Saved)"' failed

   -- regular expression does not match output --
   regexp : postfix/lmtp.* status=sent .* Saved)
   output : Sep 28 04:12:52 mail postfix/lmtp[721]: 23701B575: to=<user1@localhost.localdomain>, relay=127.0.0.1[127.0.0.1]:24, delay=0.08, delays=0.07/0/0.01/0, dsn=4.2.0, status=deferred (host 127.0.0.1[127.0.0.1] said: 451 4.2.0 <user1@localhost.localdomain> Internal error occurred. Refer to server log for more information. [2023-09-28 04:12:52] (in reply to end of DATA command))
   --

The expected pattern is logged as assert_success confirms a valid match for the log line of interest was found, and we have the mismatched value to debug the failure against.

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
  • New and existing unit tests pass locally with my changes

Instead of exit status of `124` (_signifies timeout_), it should fail with `1` (failure) like the others. Handled via using `_run_in_container_bash()` (_`timeout` failure `124` does not propagate, and is treated as `1` instead_).

In this case we are waiting on the status of the mail being sent, the pattern provided to `grep` is too specific and results in a timeout. Instead since we only expect the one log entry, match any status and assert the expected pattern afterwards.

This provides a more helpful failure output that informs us that mail was at least processed by Postfix, but the sent status is not what we expected.

### Before

```
 ✗ [ENV] (POSTFIX_DAGENT) delivers mail to existing account [60327]
   (from function `assert_success' in file test/test_helper/bats-assert/src/assert_success.bash, line 42,
    in test file test/tests/parallel/set3/mta/lmtp_ip.bats, line 47)
     `assert_success' failed

   -- command failed --
   status : 124
   output :
   --
```

### After

```
 ✗ [ENV] (POSTFIX_DAGENT) delivers mail to existing account [1425]
   (from function `assert_output' in file test/test_helper/bats-assert/src/assert_output.bash, line 178,
    in test file test/tests/parallel/set3/mta/lmtp_ip.bats, line 48)
     `assert_output --regexp "${MATCH_LOG_LINE}=sent .* Saved)"' failed

   -- regular expression does not match output --
   regexp : postfix/lmtp.* status=sent .* Saved)
   output : Sep 28 04:12:52 mail postfix/lmtp[721]: 23701B575: to=<user1@localhost.localdomain>, relay=127.0.0.1[127.0.0.1]:24, delay=0.08, delays=0.07/0/0.01/0, dsn=4.2.0, status=deferred (host 127.0.0.1[127.0.0.1] said: 451 4.2.0 <user1@localhost.localdomain> Internal error occurred. Refer to server log for more information. [2023-09-28 04:12:52] (in reply to end of DATA command))
   --
```

The expected pattern is logged as `assert_success` confirms a valid match for the log line of interest was found, and we have the mismatched value to debug the failure against.
@polarathene polarathene added service/postfix area/tests kind/improvement Improve an existing feature, configuration file or the documentation labels Sep 28, 2023
@polarathene polarathene added this to the v13.0.0 milestone Sep 28, 2023
@polarathene
Copy link
Member Author

For reference, this approach with a timeout and tail is used for the log as it's better for error output.

Other tests that run a command repeatedly until timeout via helper can get lengthy depending on output:

 ✗ [Spam - Amavis] ENV SPAMASSASSIN_SPAM_TO_INBOX (enabled + MARK_SPAM_AS_READ=1) should mark spam message as read [31308]
   (from function `_repeat_until_success_or_timeout' in file test/helper/common.bash, line 195,
    from function `_repeat_in_container_until_success_or_timeout' in file test/helper/common.bash, line 157,
    from function `_should_receive_spam_at' in file test/tests/parallel/set1/spam_virus/spam_junk_folder.bats, line 113,
    in test file test/tests/parallel/set1/spam_virus/spam_junk_folder.bats, line 92)
     `_should_receive_spam_at '/var/mail/localhost.localdomain/user1/.Junk/cur/'' failed
   [   INF   ]  mail.example.test is up and running
   grep: /var/mail/localhost.localdomain/user1/.Junk/cur/: No such file or directory
   grep: /var/mail/localhost.localdomain/user1/.Junk/cur/: No such file or directory
   grep: /var/mail/localhost.localdomain/user1/.Junk/cur/: No such file or directory
   grep: /var/mail/localhost.localdomain/user1/.Junk/cur/: No such file or directory
   grep: /var/mail/localhost.localdomain/user1/.Junk/cur/: No such file or directory
   grep: /var/mail/localhost.localdomain/user1/.Junk/cur/: No such file or directory
   grep: /var/mail/localhost.localdomain/user1/.Junk/cur/: No such file or directory
   grep: /var/mail/localhost.localdomain/user1/.Junk/cur/: No such file or directory
   grep: /var/mail/localhost.localdomain/user1/.Junk/cur/: No such file or directory
   grep: /var/mail/localhost.localdomain/user1/.Junk/cur/: No such file or directory
   grep: /var/mail/localhost.localdomain/user1/.Junk/cur/: No such file or directory
   grep: /var/mail/localhost.localdomain/user1/.Junk/cur/: No such file or directory
   grep: /var/mail/localhost.localdomain/user1/.Junk/cur/: No such file or directory
   grep: /var/mail/localhost.localdomain/user1/.Junk/cur/: No such file or directory
   grep: /var/mail/localhost.localdomain/user1/.Junk/cur/: No such file or directory
   grep: /var/mail/localhost.localdomain/user1/.Junk/cur/: No such file or directory
   grep: /var/mail/localhost.localdomain/user1/.Junk/cur/: No such file or directory
   grep: /var/mail/localhost.localdomain/user1/.Junk/cur/: No such file or directory
   Timed out on command: _exec_in_container grep -R Subject: SPAM:  /var/mail/localhost.localdomain/user1/.Junk/cur/

Copy link
Member

@georglauterbach georglauterbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏼

@polarathene polarathene merged commit 89cb6d8 into master Sep 28, 2023
7 checks passed
@polarathene polarathene deleted the tests/fix-lmtp-ip-failure-output branch September 28, 2023 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/tests kind/improvement Improve an existing feature, configuration file or the documentation service/postfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants