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

fix(cli): gracefully output configuration validation errors #772

Conversation

codejedi365
Copy link
Contributor

@codejedi365 codejedi365 commented Dec 17, 2023

Purpose

Improve handling of exceptions to provide a useful message to user without stack trace

Rationale

If a user puts in an invalid configuration setting, the configuration validator will throw a ValidationException that we do not catch gracefully. This causes python to dump a stack trace and hide the pretty ValidationError that pydantic provides by default.

In order to set up the test appropriately, I found it useful to add a fixture that will modify the configuration file accordingly. Meanwhile, I noticed that the implementation of example project was a bit fragile as exceptions are not handled well with generators or context managers unless explicitly using try/finally. With this modification I was able to align it more with the setup/teardown generator style fixtures as pytest recommends.

How I tested

Created a command line test that modifies the configuration file to be incorrect before executing the version command. It evaluates the exit code and the stderr message to the user that there is a validation error and which component.

How to verify

git fetch origin pull/772/head:pr/772

# checkout the PR as a detached HEAD state with only the unit tests
git checkout pr/772~1 --detach

# execute tests, should fail
pytest tests/ -k test_errors_when_config_file_invalid_configuration

# update to the HEAD of the PR (with fixes)
git merge --ff-only pr/772

# run pytest again (looking for success)
pytest tests/ -k test_errors_when_config_file_invalid_configuration

# run entire test suite for regression testing
PYTHONHASHSEED=123456 pytest tests/

@codejedi365 codejedi365 force-pushed the fix/config-validation-error-output branch from e198596 to b9eec65 Compare December 17, 2023 04:41
@codejedi365
Copy link
Contributor Author

@bernardcooke53, @relekang, would you mind doing a review?

Copy link
Contributor

@bernardcooke53 bernardcooke53 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @codejedi365 🎉

@bernardcooke53 bernardcooke53 merged commit e8c9d51 into python-semantic-release:master Dec 19, 2023
8 checks passed
@codejedi365 codejedi365 deleted the fix/config-validation-error-output branch December 19, 2023 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants