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

Only() not throwing an error when more validation failures are present #1986

Closed
jonmsherman opened this issue Aug 4, 2022 · 3 comments
Closed
Milestone

Comments

@jonmsherman
Copy link

jonmsherman commented Aug 4, 2022

FluentValidation version

11.1.2

ASP.NET version

5

Summary

During unit testing, I'm using Only() for a certain property and the test is still passing even when there are multiple validation failures on different properties.

Steps to Reproduce

I used the following example code to reproduce this error:

PersonDto.cs

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Client.WebService.Tests.Playground
{
    public class PersonDto
    {
        public string FirstName { get; set; }
        public string MiddleName { get; set; }
        public string LastName { get; set; }
    }
}

PersonValidator.cs

using Client.WebService.Common.Services.Interfaces;
using Client.WebService.Features.Athletes.DataAccessors.Interfaces;
using Client.WebService.Features.Athletes.Dtos.Request;
using Client.WebService.Features.Athletes.Persistence.Entities;
using FluentValidation;
using System;
using System.Globalization;
using System.Threading;
using System.Threading.Tasks;

namespace Client.WebService.Tests.Playground
{
    public class PersonValidator : AbstractValidator<PersonDto>
    {


        public PersonValidator()
        {

            RuleFor(command => command.FirstName)
                .NotEmpty()
                .WithMessage("First name is required");

            RuleFor(command => command.MiddleName)
                .NotEmpty()
                .WithMessage("Middle name is required");

            RuleFor(command => command.LastName)
                .NotEmpty()
                .WithMessage("Last name is required");
        }
    }
}

PersonValidatorTests.cs

using FluentValidation.TestHelper;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace Client.WebService.Tests.Playground
{
    [TestClass]
    public class PersonValidatorTests
    {
        private readonly PersonValidator _personValidator;
        public PersonValidatorTests()
        {
            _personValidator = new PersonValidator();
        }

        [TestMethod]
        public void Test1()
        {
            // Arrange
            var personDto = new PersonDto()
            {
                FirstName = "",
                MiddleName = "",
                LastName = "Sherman",
            };

            // Act
            var result = _personValidator.TestValidate(personDto);

            // Assert
            result
                .ShouldHaveValidationErrorFor(personDto => personDto.FirstName)
                .WithErrorMessage("First name is required")
                .Only();
        }
    }
}

If you debug this test, you will notice there's a validation error for both first name and middle name, but the test should fail because it's only expecting 1 validation error with that specific error message for FirstName

@JeremySkinner
Copy link
Member

JeremySkinner commented Aug 4, 2022

Hi @jonmsherman I believe in this case .ShouldHaveValidationErrorFor(x => x.FirstName).WithErrorMessage("First name is required").WithErrorMessage("...").Only() is essentially saying "There should only be 1 error with the message 'First name is required' ", rather than "there should only be 1 total error" and ends up filtering out the other failures.

As there is only 1 error associated with the FirstName property the test passes. The call to ShouldHaveValidationErrorFor(x => x.FirstName).WithErrorMessage("...") has already excluded those failures associated with other properties as they don't match the "WithErrorMessage" criteria

Edit to clarify: So essentially WithErrorMessage seems to act filter here which excludes the others. However the documentation seems to suggest something different should happen.

@JeremySkinner
Copy link
Member

JeremySkinner commented Aug 4, 2022

Hi @Aleksei-Pankratev-EPAM please could you take a look at this issue and confirm if it's behaving as you'd expect, as it's related to Only? Looking at the docs I think I'd expect it to work as @jonmsherman has explained.

@JeremySkinner JeremySkinner added this to the 11.2.1 milestone Aug 28, 2022
@JeremySkinner
Copy link
Member

This should be fixed as of 11.2.1, please update and give it a try.

@Aleksei-Pankratev-EPAM I'd appreciate a retroactive review of the fix (9c0c587). Only would only take into account unmatched failures in the current TestValidationContinuation, but would not consider those in a parent. My solution was to change the TestValidationContinuation to store a reference to its parent, and then Only can check the hierarchy of parents to find unmatched failures.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants