-
Notifications
You must be signed in to change notification settings - Fork 1
Simplified validation tests #181
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
base: main
Are you sure you want to change the base?
Conversation
9f13851 to
4087df9
Compare
| PartnershipType partnershipType = limitedPartnershipDto.getData().getPartnershipType(); | ||
|
|
||
| String contributionCurrencyValue = limitedPartnerDataDto.getContributionCurrencyValue(); | ||
| String contributionCurrencyValue = limitedPartnerDataDto.getContributionCurrencyValue().toString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ignore... just to get it to compile
| import org.springframework.validation.Validator; | ||
|
|
||
| @SpringBootTest | ||
| class LimitedPartnerDataDtoValidationTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idea based on what was spoke about in dev meeting yesterday. Simplify test for validation, which includes fluid api builder so its clear on the preconditions for any validation scenario.
This idea could be replicated for the LimitedPartnershipValidation class as well..... then you could have one controller test to validate javax validation is configured correctly. Removing the complexity of setting up json with valid/ invalid values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we did follow this validation approach early on, but shelved it for the 'more complete' tests driven by the controller testing. See LimitedPartnershipDtoValidationTest.
I think you're doing essentially the same thing with this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, pretty much... I suppose the better example would be to use the custom validator that works on multiple objects. Find the controller tests hard to read, so much going on.... but perhaps thats just me
JIRA link
Change description
Work checklist
Pull request checklist
An exhaustive list of peer review checks can be read here
Merge instructions
We are committed to keeping commit history clean, consistent and linear. To achieve this commit should be structured as follows:
and contain the following structural elements:
BREAKING CHANGE:introduces a breaking API change (correlating with MAJOR in semantic versioning). A BREAKING CHANGE can be part of commits of any type,fix:andfeat:are allowed, for examplebuild:,chore:,ci:,docs:,style:,refactor:,perf:,test:, and others,BREAKING CHANGE: <description>may be provided.