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

Adding bUnit tests #218

Merged
merged 9 commits into from
May 28, 2024
Merged

Adding bUnit tests #218

merged 9 commits into from
May 28, 2024

Conversation

luetm
Copy link
Contributor

@luetm luetm commented Feb 21, 2024

Ok, this is what I have written so far. I have come across some issues, on which I need your feedback to.


Rule sets applied inconsistently

When validators contain rule sets, but these rule sets are not being included manually:

  • Validate and ValidateAsync seem to ignore them
  • Validation messages are still triggered in the UI

Is this intended behavior?


Validation options

The syntax provided from the readme seems to be broken:

<FluentValidationValidator Options="@(options => options.IncludeRuleSets("Names"))" />

When I add this I get compiler messages, and I was unable to fix them by fiddling with the syntax or adding a private void IncludeNames(ValidationStrategy<object> options) { ... }. Adding them by code works. The message is about the FluentValidation.Internals namespace not existing.


GetFailuresFromLastValidation

To my shame, GetFailuresFromLastValidation() only seems to work when using ValidateAsync, not when using Validate. 2 tests are failing because of this, which is intended. The problem is, that only ValiadteAsync() adds the validation failures to the LastValidationResult property. Is it possible to add this with Validate or should we just add it as a caveat to the readme?


Please feel free to point out other issues I should test, or changes I should make!

@luetm luetm mentioned this pull request Feb 22, 2024
@luetm
Copy link
Contributor Author

luetm commented Feb 22, 2024

Conclusion of #216

@luetm
Copy link
Contributor Author

luetm commented Mar 4, 2024

@chrissainty Is this the way you imagined it?

Copy link
Collaborator

@pwelter34 pwelter34 left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for contribution

@pwelter34
Copy link
Collaborator

@luetm If you can update your branch, i can merge this. Thanks

@pwelter34 pwelter34 merged commit 5b75236 into Blazored:main May 28, 2024
1 check passed
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.

2 participants