tests(refactor): mail_hostname.bats#3027
Merged
polarathene merged 20 commits intodocker-mailserver:masterfrom Jan 29, 2023
Merged
tests(refactor): mail_hostname.bats#3027polarathene merged 20 commits intodocker-mailserver:masterfrom
mail_hostname.bats#3027polarathene merged 20 commits intodocker-mailserver:masterfrom
Conversation
Based on changes from an earlier closed hostname PR from Oct 2021 with additional revision to use `assert_output` and more thorough checking of values expected in output.
This was originally a single test case in `tests.bats` intended for `supervisord` testing. It seems at some point it got reassigned to a hostname override test container, and then migrated to separate test file for hostname override test by accident. It now belongs in the correct place again, as hostname config should have nothing to do with a graceful shutdown?
Wait for SMTP port is left at the end to avoid additional start-up delays.
- I could do multiple container grep calls instead, but opted to match by lines in file. This better ensures values are being matched to the correct lines. - Renamed the test case descriptions. - Expanded test coverage of the 4th container as it represents another DNS config, while the 3rd is just the 4th container with the `SRS_DOMAINNAME` env added, no value in more coverage there.
These checks are performed in `mail_hostname.bats` with better coverage.
The original `fqdn-with-subdomain` is now `with-nis-domain` which is more accurate. A new test case will properly cover the default `--hostname` only config that is not a bare domain.
This commit just shifts the test cases, no new changes to any content beyond that.
georglauterbach
requested changes
Jan 26, 2023
Member
georglauterbach
left a comment
There was a problem hiding this comment.
LGTM 👍🏼 Some minor remarks left.
Co-authored-by: Georg Lauterbach <[email protected]>
- Fix a suggested change bug with quote wrapping an interpolated variable. - Convert two other `_bash` methods that were missed from review. - Apply the last two suggested changes from review.
8ff1bb1 to
07877a6
Compare
The `| head -n 1` can be dropped if we know for sure it's only one line, which is what we expect. Quotes can then be dropped too.
Member
Author
|
All test feedback resolved, including suggestions you wanted verified 👍 |
casperklein
approved these changes
Jan 29, 2023
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
mail_hostname.batshas been converted into the new testing format and completely refactored to share common tests by grouping them into functions, improved inline docs and expanded test coverage.There was some overlap in
tests.bats, where coveraage is better inmail_hostname.bats. Additionally the tests for graceful shutdown seems like they were misplaced, but I have now relocated back into thesupervisordtests (the check was originally added in Nov 2018, and additionally covers the grace period added in May 2021?).For reference, I originally did a thorough review of the tested network hostname configs and logic changes, refactoring and extracting out the larger original PR into smaller ones in Nov 2021.
Commit history scopes the refactor progress if that's helpful for tracking changes vs the diff alone.
Type of change
Checklist: