Skip to content

chore: Remove the Makefile backup target#3000

Merged
polarathene merged 5 commits intodocker-mailserver:masterfrom
polarathene:chore/makefile-remove-backup
Jan 12, 2023
Merged

chore: Remove the Makefile backup target#3000
polarathene merged 5 commits intodocker-mailserver:masterfrom
polarathene:chore/makefile-remove-backup

Conversation

@polarathene
Copy link
Copy Markdown
Member

@polarathene polarathene commented Jan 12, 2023 •
edited
Loading

Description

This is no longer serving any value to us. It was made redundant with changes added in Oct 2020.

  • The .gitignore entries were added in July 2019. They are no longer created by our tests.
  • A new method to duplicate a base config folder for tests to use was introduced in Oct 2020. Part of the early test extraction efforts.
  • History of the Makefile backup target is detailed in a comment from me in March 2022. It was introduced in 2016. The linked comment discussed removing the backup target entirely (I stumbled upon it and figured I should raise a PR for it).
  • no_container.bats was a test creating a bunch of paths, but did not seem all that useful as setup-cli.bats covers the bulk of functionality tested. Stripped it down to the only relevant test case.

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

@polarathene
chore: Remove backup target from Makefile
5897535 
- No longer relevant.
- `clean` target inline docs revised.
- `.gitignore` remove test lines that are no longer valid.
@polarathene
chore: Parallel test target split to multi-line
bed75e5
@polarathene polarathene added area/tests kind/improvement Improve an existing feature, configuration file or the documentation labels Jan 12, 2023
@polarathene polarathene added this to the v12.0.0 milestone Jan 12, 2023
@polarathene polarathene self-assigned this Jan 12, 2023
@polarathene polarathene marked this pull request as draft January 12, 2023 04:28
@polarathene
Copy link
Copy Markdown
Member Author

polarathene commented Jan 12, 2023 •
edited
Loading

Tests were still running while I opened up the PR. I did encounter some failures. The Makefile should probably trigger test suite, since this didn't raise any signals of breakage it'd incur? (EDIT: I've added it as a trigger for the testing workflow)

@polarathene
tests(fix): Test setup.sh with temporary config dir
c1b31aa 
This test has many redundant test cases already covered by `setup-cli`. They're basically identical. Removed all but one.

This removes some config dirs that were being explicitly created instead of using the test helper to generate a directory that can be used to test the `-p` option instead.
@polarathene
ci: Ensure tests are run when Makefile is modified
a515e02
@polarathene polarathene marked this pull request as ready for review January 12, 2023 06:32
@polarathene
Copy link
Copy Markdown
Member Author

polarathene commented Jan 12, 2023 •
edited
Loading

@georglauterbach I know you said you wanted to tackle no_container.bats but it affected this PR 😅

From what I could see that test was largely redundant (as setup-cli.bats covers setup command). The only worthwhile test left was to ensure that -p and -i options of setup.sh worked correctly. I've taken care of that here.

@georglauterbach
Copy link
Copy Markdown
Member

georglauterbach commented Jan 12, 2023

@georglauterbach I know you said you wanted to tackle no_container.bats but it affected this PR 😅

From what I could see that test was largely redundant (as setup-cli.bats covers setup command). The only worthwhile test left was to ensure that -p and -i options of setup.sh worked correctly. I've taken care of that here.

That's alright, less work for me :D

@polarathene polarathene merged commit 1650cdf into docker-mailserver:master Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@casperklein casperklein casperklein approved these changes

@georglauterbach georglauterbach georglauterbach approved these changes

Assignees

@polarathene polarathene

Labels

area/tests kind/improvement Improve an existing feature, configuration file or the documentation

Projects

None yet

Milestone

v12.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@polarathene @georglauterbach @casperklein