style: minor adjustments

[georglauterbach]

Description

These adjustments are just follow-ups for the latest PRs I merged. If you have more adjustments, feel free to add them to this PR.

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

[polarathene]

I didn't expect |& to have much luck with search engines, but I stand corrected 😅 (I did see the helpful commit message prior to searching, but was curious)

I suppose we have a lot of bash specific syntax throughout the project that a shell like ash probably isn't compatible?

[casperklein]

I don't know exactly, how compatible ash/dash etc. are to bash. But as we wrote our scripts for bash, I am very fine with using bash specific syntax to unleash the full power 😆

[casperklein]

@docker-mailserver/maintainers While we at it: what about replacing the sed command with:

grep -oP '(?<=Primary key fingerprint: ).+'

Regex explanation: The ?<= within the group means --> ignore this group and match only .+ that comes after.

This seems simpler and easier read (for me at least).

But no need for a lengthy discussion IMO. Just give a 👎 to keep it as it is, or 👍 to go with grep.

[polarathene]

I don't think it's worth it personally.

Very minor improvement, and not something I'm going to remember, or like looking up if I saw it months later and forgot (git blame aside).

The sed command is familiar enough, I've seen the usage here elsewhere in the project, so it's easier to maintain.

[georglauterbach]

I do not see an inherent need for this change as well - doesn't really matter I think. I'd just like to merge this sometime soon so we can get a feature freeze in place for v11.2.0 :)

[polarathene]

@casperklein currently tests failed with the healthcheck, which I think you tried to address with a recent PR:

not ok 219 checking container healthcheck in 115ms
# (from function `assert_output' in file test/test_helper/bats-assert/src/assert_output.bash, line 194,
#  in test file test/tests.bats, line 123)
#   `assert_output "healthy"' failed
#
# -- output differs --
# expected : healthy
# actual   : starting
# --
#

[casperklein]

I've provided a possible fix for the health check in #2810.

Afaik, that was the first time, that the health check failed since I addressed it recently? More annoying is however the test #217 which pretty often fails. This forces me sometimes to restart the tests three or more times.

[georglauterbach]

We will definitely need to do something about the issue where only 217 tests run. The setup_file function for tests.bats fails, which leads to all the tests in this big file not being run. I will have a look (but do this in another PR).

[polarathene]

When you both experienced the test 217 failures, I assume it was on PRs that would not result in invalidating the ClamAV cache, and that cache may be old enough to have ClamAV update when the container runs?

If so I have noticed on my system how problematic that can be for getting tests.bats to be successful.

[georglauterbach]

When you both experienced the test 217 failures, I assume it was on PRs that would not result in invalidating the ClamAV cache, and that cache may be old enough to have ClamAV update when the container runs?

Yes, this is absolutely possible.

If so I have noticed on my system how problematic that can be for getting tests.bats to be successful.

Interesting - I will have a look into this too. Will provide more information in #2811.