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

feat: Prevent unused flags #4182

Merged
merged 4 commits into from
Jan 29, 2024
Merged

Conversation

lukemassa
Copy link
Contributor

@lukemassa lukemassa commented Jan 28, 2024

what

Add a unit test to make sure that all the values in server.UserConfig appear in server_test.testFlags

why

When you add a flag you're supposed to add it to four places: cmd/server.go as a const, cmd/server.go as part of a map for flags, cmd/server_test.go to test, and the configuration struct in server/server.UserConfig. Per #4063, if you forget some combination of these a unit test already fails (example: https://github.com/runatlantis/atlantis/blob/v0.27.1/cmd/server_test.go#L124). This adds an additional unit test to make sure a different misconfiguration doesn't occur.

tests

Ran unit tests.

references

Partially addresses: #4063
Prevents recurrence of issues like: #4183, #4064

@lukemassa lukemassa marked this pull request as ready for review January 29, 2024 18:59
@lukemassa lukemassa requested review from a team as code owners January 29, 2024 18:59
@lukemassa lukemassa requested review from chenrui333, nitrocode and X-Guardian and removed request for a team January 29, 2024 18:59
@github-actions github-actions bot added the go Pull requests that update Go code label Jan 29, 2024
jamengual
jamengual previously approved these changes Jan 29, 2024
Copy link
Contributor

@jamengual jamengual left a comment

Choose a reason for hiding this comment

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

Great Idea, thanks @lukemassa

@GenPage GenPage added the feature New functionality/enhancement label Jan 29, 2024
@lukemassa
Copy link
Contributor Author

The CI wasn't running properly so I pushed a small update (cleaned up the verbiage slightly). Merging as is.

@lukemassa lukemassa merged commit fad38c5 into runatlantis:main Jan 29, 2024
24 checks passed
@chenrui333 chenrui333 added this to the v0.28.0 milestone May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality/enhancement go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants