Skip to content

Conversation

@sisp
Copy link
Member

@sisp sisp commented May 15, 2025

I've fixed a bug that prevented default answers from being validated against the validator expression.

Before, using a default answer through --defaults did not apply validation. Now, the Question.get_default() method not only casts the answer but also validates it. Initially, this change caused an infinite recursion because Question._formatted_choices used the default answer to pre-select choices for multi-select questions and Question.parse_answer() uses Question._formatted_choices to retain the order of choice items in a multi-select question answer, so there was a cycle. To avoid this cycle, I've moved the snippet for pre-selecting choices to Question.get_questionary_structure(), as it's only important to add this information for prompting. As Question._formatted_choices is a cached property, I've decided to deep-copy this list of Choice objects to avoid mutating them, risking potentially hard-to-debug side effects.

As it turned out, one test case I added to tests/test_copy.py::test_validate_default_value in #1770 was silently incorrect and would have caught this bug already back then. 🫣 This is fixed now as well.

To avoid duplicate error logic and message code, Question.validate_answer() now raises a ValueError when validation fails instead of returning the error message string. This way, we raise the same exception wherever answer validation fails.

Hands down, this isn't pretty, but it works for now.

Fixes #2143.

@codecov
Copy link

codecov bot commented May 15, 2025

Codecov Report

❌ Patch coverage is 95.23810% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 98.03%. Comparing base (608b5d4) to head (a1d47fd).
⚠️ Report is 100 commits behind head on master.

Files with missing lines Patch % Lines
copier/_user_data.py 94.44% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2145      +/-   ##
==========================================
+ Coverage   98.01%   98.03%   +0.01%     
==========================================
  Files          55       55              
  Lines        5949     5954       +5     
==========================================
+ Hits         5831     5837       +6     
+ Misses        118      117       -1     
Flag Coverage Δ
unittests 98.03% <95.23%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sisp sisp merged commit 8bf99c7 into master May 20, 2025
20 of 21 checks passed
@sisp sisp deleted the fix/validate-default-answers branch May 20, 2025 08:18
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.

validator checks are skipped for questions with a default when --defaults option is set

2 participants