search

jakartaee / validation-tck

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

BVTCK-32 First cut of BV 1.1 TCK tests #8

Closed gunnarmorling closed 11 years ago

gunnarmorling commented 11 years ago

As discussed with Hardy, I'm going to create several pull requests for the TCK tests in order to avoid reviewing one huge request. Note that this request contains some tests currently failing, as the RI needs to be updated/fixed.

hferentschik commented 11 years ago

looking ...

gunnarmorling commented 11 years ago

Cool, let me know anything is odd, not correct etc. This PR already got larger than I had hoped, sigh.

FYI, according to the coverage tool this ca. the half of the new tests for BV 1.1 (that is, existing assertions to be updated for 1.1 are not included in that number).

hferentschik commented 11 years ago

if this is already half the new test that would be great.

gunnarmorling commented 11 years ago

Maybe I also should say half of the assertions, that is, maybe we have to add more tests to individual assertions :)

hferentschik commented 11 years ago

On a tangent - did you ever look at the TCK docs to verify the setup and procedures - http://docs.jboss.org/hibernate/beanvalidation/tck/1.1/reference/html_single?

hferentschik commented 11 years ago

I think we should deploy TCK SNAPSHOTS and run them in the TCK runner module of HV. I guess that HV-670. I'll have a loot at it.

gunnarmorling commented 11 years ago

On a tangent - did you ever look at the TCK docs to verify the setup and procedures - http://docs.jboss.org/hibernate/beanvalidation/tck/1.1/reference/html_single?

Yes, I used it to get started; I haven't done a thorough review, though. Basically everything worked as intended, but there are some TODOs left. I can do a review in the context of BVTCK-23.

gunnarmorling commented 11 years ago

I think we should deploy TCK SNAPSHOTS and run them in the TCK runner module of HV. I guess that HV-670. I'll have a loot at it.

Yeah, right. I used the runner to execute the tests locally and in AS. For the latter, HV-670 is required.

hferentschik commented 11 years ago

Could you confirm that the following tests - https://github.com/hferentschik/beanvalidation-tck/tree/ri-failing-tests - also fail on the RI? We also should specify the group in tck-tests.xml. Maybe you can add this commit to this pull request?

gunnarmorling commented 11 years ago

Could you confirm that the following tests - https://github.com/hferentschik/beanvalidation-tck/tree/ri-failing-tests - also fail on the RI?

The tests marked in your first commit do indeed fail for me, too (I could swear I had marked them already, but apparently this got somehow lost), but the other two from your second commit run green for me. Which error do you get for these?

gunnarmorling commented 11 years ago

Added commits for incorporating review remarks and marking more tests as currently failing on the RI

hferentschik commented 11 years ago

On a tangent - did you ever look at the TCK docs to verify the setup and procedures - http://docs.jboss.org/hibernate/beanvalidation/tck/1.1/reference/html_single?

Yes, I used it to get started; I haven't done a thorough review, though. Basically everything worked as intended, but there are some TODOs left. I can do a review in the context of BVTCK-23.

Good. A lot of the todos are final version numbers. Most importantly you did not hit a majot roadblock. We will clean everything up before the release as part of BVTCK-23

hferentschik commented 11 years ago

Not sure where the relevant part of the discussion went, but lets stick to consistent camel cases. If I ever have a little time I try the script I have in mind and we can compare. Even if it is not Java style it might improve readability.

gunnarmorling commented 11 years ago

but the other two from your second commit run green for me.

Turns out this is an issue with the RI, too (also the spec needs to be updated). The tests fails on AS due to method parameters passed to the JPA traversable resolver. I've created HV-673. I've added both commits to this PR.