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

TestHelper.ShouldHaveValidationErrorFor.WithoutErrorCode broken in 11.0 #1937

Closed
JothanChanes opened this issue May 5, 2022 · 10 comments · Fixed by #1938
Closed

TestHelper.ShouldHaveValidationErrorFor.WithoutErrorCode broken in 11.0 #1937

JothanChanes opened this issue May 5, 2022 · 10 comments · Fixed by #1938
Milestone

Comments

@JothanChanes
Copy link

FluentValidation version

11.0

ASP.NET version

No response

Summary

When adding multiple validation rules the TestHelper's ShouldHaveValidationErrorFor.WithoutErrorCode throws for each and any specified error code (see MWE below). The issue was introduced by d4773d2, but I'm not yet(?) powerful enough to suggest a fix.

Steps to Reproduce

namespace FluentValidation.Tests {

	using Xunit;
	using FluentValidation.TestHelper;
	public class WithoutErrorCodeIssue_Reproduce {

		class A {
			public A() { s = ""; }
			public double d = 0;
			public string s;
		}

		class AV : AbstractValidator<A> {
			public AV() {
				RuleFor(a => a.d).InclusiveBetween(1, 3); // remove this line this line to make the test pass
				RuleFor(a => a.s).NotEmpty();
			}
		}

		[Fact]
		public void Reproduce() {
			var vld = new AV();
			var res = vld.TestValidate(new A());

			res.ShouldHaveValidationErrorFor(a => a.s).WithoutErrorCode("abc");
		}
	}
}
@JothanChanes JothanChanes changed the title TestHelper.ShouldHaveValidationErrorFor.WithoutErrorCode broken TestHelper.ShouldHaveValidationErrorFor.WithoutErrorCode broken in 11.0 May 5, 2022
@thomOrbelius
Copy link

thomOrbelius commented May 5, 2022

I am also experiencing this issue. It applies on ShouldHaveValidationErrorFor in general, either with or without .WithoutErrorCode.

I started my application and when I called the endpoint with validation it listed all expected validation errors.

@JeremySkinner
Copy link
Member

Hi @Aleksei-Pankratev-EPAM are you able to help with this as it's related to the introduction of Only()?

@Aleksei-Pankratev-EPAM
Copy link
Contributor

@JeremySkinner yes, I will have a look.

@JeremySkinner
Copy link
Member

Brilliant, thanks!

@Aleksei-Pankratev-EPAM
Copy link
Contributor

#1938 should fix this but I would ask to hold on with merging it as I want to check more edge cases.

@JeremySkinner JeremySkinner added this to the 11.0.1 milestone May 5, 2022
@akamud
Copy link
Contributor

akamud commented May 6, 2022

I'm seeing a very similar behavior happening to the extensions WithErrorMessage, I believe they have the same root cause.

validator.TestValidate(dto).ShouldHaveValidationErrorFor(x => x.Status).WithErrorMessage("A");

This throws a message from another property (Name). If I downgrade to 10.4 it works as expected: showing the error message from Status property. I believe #1938 will fix it, but this might be a test case to add.

@Aleksei-Pankratev-EPAM
Copy link
Contributor

@akamud I cannot reproduce your case. This is passing:

		[Fact]
		public void Expected_with_error_code_check() {
			var validator = new InlineValidator<Person> {
				v => v.RuleFor(x => x.Forename).NotNull(),
				v => v.RuleFor(x => x.Surname).NotNull()
					.WithMessage("bar")
			};

			validator.TestValidate(new Person())
				.ShouldHaveValidationErrorFor(x => x.Surname)
				.WithErrorMessage("bar");
		}

Can you prepare a failing test for I could double check if #1938 fixes it?

@akamud
Copy link
Contributor

akamud commented May 6, 2022

@akamud I cannot reproduce your case. This is passing:

		[Fact]
		public void Expected_with_error_code_check() {
			var validator = new InlineValidator<Person> {
				v => v.RuleFor(x => x.Forename).NotNull(),
				v => v.RuleFor(x => x.Surname).NotNull()
					.WithMessage("bar")
			};

			validator.TestValidate(new Person())
				.ShouldHaveValidationErrorFor(x => x.Surname)
				.WithErrorMessage("bar");
		}

Can you prepare a failing test for I could double check if #1938 fixes it?

I'm sorry, I should have been more specific. This is a failing test for this scenario:

public class Person
{
    public string? Forename { get; set; }
    public int? Surname { get; set; }
}

public class Tests
{
    [Test]
    public void Test1()
    {
        var validator = new InlineValidator<Person>
        {
            v => v.RuleFor(x => x.Forename).NotEmpty(), v => v.RuleFor(x => x.Surname).NotEmpty()
        };

        validator.TestValidate(new Person())
            .ShouldHaveValidationErrorFor(x => x.Surname)
            .WithErrorMessage("bar");
    }
}

You can see the test failing with the Forename error instead of the Surname:

FluentValidation.TestHelper.ValidationTestException : Expected an error message of 'bar'. Actual message was ''Forename' must not be empty.'

Update:
I think the issue is the error message itself, not the test. If I pass the correct message ('Surname' must not be empty.), the test passes.

Downgrading to 10.4 fixes this issue.

@Aleksei-Pankratev-EPAM
Copy link
Contributor

Thank you @akamud! Now this is fixed too.

JeremySkinner pushed a commit that referenced this issue May 7, 2022
…failures (#1937) (#1938)

* Apply validation predicate to matched entries only for Without*** worked correctly when there is more than one failure
* Apply validation predicate only to matched entries for With*** assertions too to correctly display failures
@JeremySkinner
Copy link
Member

I've pushed out 11.0.1 to nuget with the fix. Thanks @Aleksei-Pankratev-EPAM for your work on this

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