Open
Conversation
…re instead of all failures
Add ChineseTraditional language
jarlungoodoo73
added
bug
Reviewer's GuideUpdates test helper validation error filtering to only require at least one matching failure, adds a regression test for multiple error matches, introduces Traditional Chinese (zh-TW) localization support, bumps the build script version, and wires the new language into the language manager. Class diagram for new ChineseTraditionalLanguage and LanguageManager updateclassDiagram
class Language {
}
class ChineseTraditionalLanguage {
+string Name
+ChineseTraditionalLanguage()
}
class LanguageManager {
+LanguageManager()
-Language[] languages
}
ChineseTraditionalLanguage --|> Language
LanguageManager o--> Language
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The new implementation of
Whennow callsAnyand thenFirst, which will enumerate theIEnumerable<ValidationFailure>multiple times; consider materializing to a list or using a single pass to avoid unexpected behavior with non-replayable enumerables. - The updated
Whenlogic assumes there is at least one failure (usingFirst()); previously an emptyfailurescollection would simply return without throwing, so you may want to either handle the empty case explicitly or confirm this behavior change is intentional.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new implementation of `When` now calls `Any` and then `First`, which will enumerate the `IEnumerable<ValidationFailure>` multiple times; consider materializing to a list or using a single pass to avoid unexpected behavior with non-replayable enumerables.
- The updated `When` logic assumes there is at least one failure (using `First()`); previously an empty `failures` collection would simply return without throwing, so you may want to either handle the empty case explicitly or confirm this behavior change is intentional.
## Individual Comments
### Comment 1
<location> `src/FluentValidation/TestHelper/ValidatorTestExtensions.cs:124-127` </location>
<code_context>
- foreach (var failure in failures) {
- if (!failurePredicate(failure)) {
- string message = "Expected validation error was not found";
+ bool anyMatched = failures.Any(failurePredicate);
- if (exceptionMessage != null) {
- message = exceptionMessage.Replace("{Code}", failure.ErrorCode)
- .Replace("{Message}", failure.ErrorMessage)
- .Replace("{State}", failure.CustomState?.ToString() ?? "");
- }
+ if (!anyMatched) {
+ var failure = failures.First();
+
+ string message = "Expected validation error was not found";
</code_context>
<issue_to_address>
**issue (bug_risk):** Handling empty failure sequences will throw an unexpected InvalidOperationException instead of the intended ValidationTestException.
When `failures` is empty, `Any` returns `false`, so the `if (!anyMatched)` branch executes, and `failures.First()` throws `InvalidOperationException` before the intended `ValidationTestException`. Previously this method returned quietly in the no-failures case (with earlier helpers asserting on failure presence). To preserve the expected behavior and exception type, add an explicit empty-sequence guard (e.g., use `FirstOrDefault()` and handle `null`, or materialize to a list and check `Count == 0` before accessing the first element).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+124
to
+127
| bool anyMatched = failures.Any(failurePredicate); | ||
|
|
||
| if (exceptionMessage != null) { | ||
| message = exceptionMessage.Replace("{Code}", failure.ErrorCode) | ||
| .Replace("{Message}", failure.ErrorMessage) | ||
| .Replace("{State}", failure.CustomState?.ToString() ?? ""); | ||
| } | ||
| if (!anyMatched) { | ||
| var failure = failures.First(); |
There was a problem hiding this comment.
issue (bug_risk): Handling empty failure sequences will throw an unexpected InvalidOperationException instead of the intended ValidationTestException.
When failures is empty, Any returns false, so the if (!anyMatched) branch executes, and failures.First() throws InvalidOperationException before the intended ValidationTestException. Previously this method returned quietly in the no-failures case (with earlier helpers asserting on failure presence). To preserve the expected behavior and exception type, add an explicit empty-sequence guard (e.g., use FirstOrDefault() and handle null, or materialize to a list and check Count == 0 before accessing the first element).
There was a problem hiding this comment.
Pull request overview
This PR includes a bug fix for the validation test helper extension method When() and adds Chinese Traditional language translation support. The changes prepare version 7.6.105 for release.
- Fixes issue FluentValidation#858 where
ValidationTestExtension.When()incorrectly checked each failure individually instead of checking if any failure matches the predicate - Adds complete Chinese Traditional (zh-TW) language translation support
- Updates version number from 7.6.104 to 7.6.105
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/FluentValidation/TestHelper/ValidatorTestExtensions.cs | Refactored When() method to use Any() for checking if any failure matches the predicate, rather than iterating through each failure individually |
| src/FluentValidation/Resources/Languages/ChineseTraditionalLanguage.cs | Added new language file with Traditional Chinese translations for all validator error messages |
| src/FluentValidation/Resources/LanguageManager.cs | Registered the new ChineseTraditionalLanguage in the language manager's constructor |
| src/FluentValidation.Tests/ValidatorTesterTester.cs | Added test case to verify that multiple validation errors can be independently matched using the fluent API |
| build.ps1 | Bumped version number from 7.6.104 to 7.6.105 |
| Changelog.txt | Added release date and documented the new features and bug fix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| var person = new Person() { Surname = "c" }; | ||
| var result = validator.TestValidate(person); | ||
|
|
There was a problem hiding this comment.
Inconsistent whitespace: there are multiple spaces before the statement end. This should be a single space or no space before the semicolon for consistency with the rest of the codebase.
Suggested change
Comment on lines
+375
to
+381
| var validator = new InlineValidator<Person> { | ||
| v => v.RuleFor(x => x.Surname).Equal("a").WithErrorCode("nota"), | ||
| v => v.RuleFor(x => x.Surname).Equal("b").WithErrorCode("notb") | ||
| }; | ||
|
|
||
| var person = new Person() { Surname = "c" }; | ||
| var result = validator.TestValidate(person); |
There was a problem hiding this comment.
Local scope variable 'validator' shadows ValidatorTesterTester.validator.
Suggested change
| var validator = new InlineValidator<Person> { | |
| v => v.RuleFor(x => x.Surname).Equal("a").WithErrorCode("nota"), | |
| v => v.RuleFor(x => x.Surname).Equal("b").WithErrorCode("notb") | |
| }; | |
| var person = new Person() { Surname = "c" }; | |
| var result = validator.TestValidate(person); | |
| var inlineValidator = new InlineValidator<Person> { | |
| v => v.RuleFor(x => x.Surname).Equal("a").WithErrorCode("nota"), | |
| v => v.RuleFor(x => x.Surname).Equal("b").WithErrorCode("notb") | |
| }; | |
| var person = new Person() { Surname = "c" }; | |
| var result = inlineValidator.TestValidate(person); |
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.
Create a new pull request by comparing changes across two branches. If you need to, you can also
Summary by Sourcery
Add Traditional Chinese localization support, refine validation test failure matching behavior, and bump the build version.
New Features:
Bug Fixes:
Build:
Tests: