Description

This refactoring required a bit of extra investigation into the feature under test. That revealed some redundant content to drop, detailed below.

Refactor summary

• Converted to new testing conventions and common container helpers.
• ENABLE_POSTFIX_VIRTUAL_TRANSPORT was not relevant, dropped. Only POSTFIX_DAGENT matters for the feature.
• Revised test cases, logic remains the same.
• Large custom config used was not documented and doesn't appear to serve any purpose. Simplified by replacing with a single modification with user-patches.sh.
• Added some additional comments for context of test and improvements that could be made.

NOTE: The ENV removal here is not a breaking change. So long as the actual ENV with LMTP URI is present, it continues to work (unless someone weirdly relied on keeping that URI var and toggling the enable var instead). It is part of v12 and changelog will still highlight it regardless if some third-party integrations were relying upon it.

The feature itself seems rather niche. You'd still need to manually configure DMS like the user-patches.sh is doing to leverage it with another DMS instance. The value of this ENV is for services with LMTP that aren't DMS based AFAIK? It might be relevant to deprecate the ENV and drop it in the future (deferring to postfix-main.cf or similar) if rarely used?

Additional info

Background (History)

• Originally contributed Jan 2017.
• Initially contributor began work on this support in Sep 2016, around this time in Sep 2016 there was already discussion to switch to LMTP in the project and this was merged in Oct 2016.
• The feature tested was implemented with two ENV, instead of using the postfix-main.cf override feature (available since Mar 2016). Using ENV instead was justified by the contributor as a convenience to skip providing a config file.
• Feature was approved and merged by original DMS author, despite the massive config dir thrown in for tests (lacking much documentation or explanation for what it was doing differently). One of the ENV also seemed rather pointless? (the only usage of it appears to be conditionally run the main.cf update with the value of the 2nd ENV).

Removing entire config dir for this test in favor of user-patches.sh

I've identified none of it was relevant to the test except a few files. Only one of those actually had an effect.

Only relevant changes in test config regarding LMTP:

• /etc/dovecot/dovecot.conf (protocols = imap lmtp) and /etc/dovecot/protocols.d/ (protocols = $protocols lmtp).
• conf.d/10-master.conf only changed the LMTP service listener from a unix socket to TCP on port 24 (This was the only change required for the test to pass).

None of those configs are required as:

• protocols = imap pop3 lmtp is the upstream default, no need to add lmtp.
• The LMTP service listener is now configured for the test with user-patches.sh.

Misc changes

Removed comment from mail_hostname.bats

In an old PR I was not familiar with LMTP or the relevance of the comment in this test file. I lacked time back then to investigate to know if it was ok to remove it 😅

There was a comment 'postfix virtual transport lmtp' followed by two lines sending mail. This came from this commit
(from a PR in Oct 2020).

There's no value from that comment, as the email sent is for generic testing, nothing to do with LMTP (Likely transferred over from test.bats at the time by accident).

smtp-delivery.bats

Similar logic to match delivery logs. Figured I'd keep that in sync, and reduced the horizontal scroll for the command by extracting out the matching and formatting parts into vars. Added an example comment if helpful.

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