jakartaee / validation-tck

Jakarta Validation TCK
http://validator.hibernate.org
Apache License 2.0
38 stars 33 forks source link

BVTCK-178 Using new assert framework where possible #144

Closed marko-bekhta closed 6 years ago

marko-bekhta commented 7 years ago

Same thing as for HV tests - replaced usage of some methods with

assertThat( constraintViolations ).containsOnlyViolations(
    violationOf( SomeConstraint.class )
....
);

I also have run HV build against this updated TCK version - all tests passed.

gsmet commented 7 years ago

Wow. This is huge and a very good thing, thanks for that.

We are in the final stretch for releasing BV 2.0 (we submit the final artifacts on Tuesday) so it's a bit late to merge this one for this release. I marked it for 2.1, we will merge this as soon as we're done with the 2.0 release.

What would be interesting now that we use it everywhere is to have a regression test for the testing utility itself i.e. triggering a violation and validating that for each tested element (path, constraint...), we have an error if we compare to an incorrect value.

Typically, if it wasn't testing anything, we would currently have everything green, whereas the whole purpose of the TCK would be defeated.

Do you think you could add this test? It shouldn't be too hard nor too long.

marko-bekhta commented 7 years ago

Hi @gsmet, yes I'll add the tests for this utility. Should it be a separate PR, or can it go together with this one ?

As for the merging part - I think it'll be fine, as you've said it's almost final so there will be no huge changes, so no conflicts later when merging this, right ? :)

gsmet commented 7 years ago

Yes, basically, I have a few tests to add regarding a clarification of the getLeafBean method for container elements and we're done.

We might have to make changes if other implementations detect issues in the TCK but it should be limited so I don't expect any merge issue.

Fingers crossed :)

gsmet commented 7 years ago

@marko-bekhta sorry missed you question. Depends :). If it arrives before we cut the final, make it a separate issue and PR. as I'll probably include it in this release.

gunnarmorling commented 7 years ago

@gsmet We can probably move forward with that one, now?

gsmet commented 7 years ago

@gunnarmorling yes, don't worry, I have it on the radar. Sharing my time between OGM and HV now.

gunnarmorling commented 7 years ago

Cool, thanks!

gsmet commented 6 years ago

Forgot to close this one.