Hi,

2018-03-13 11:30 GMT+01:00 Guillaume Smet <guillaume.smet@gmail.com>:

Hi Matt,

On Tue, Mar 13, 2018 at 12:18 AM, Matt Benson <mbenson@apache.org> wrote:
Hi all,
This issue could be similar to my last one. On #testValidOnListAndOnTypeArgumentWithGroupConversions() the expectation is the single violation on visitors[0].name with the Default group. My implementation converts Default to ExtendedChecks1, cascades the legacy list elements, then converts ExtendedChecks1 to ExtendedChecks2 when cascading the type parameters. I end up with violations on visitors[0].extended1 and visitors[0].extended2 . I don't see any interpretation that seems to justify triggering the violation on the Default group. The test points to spec section 5.1.3 but a reread of that section does nothing for me here. Help would be appreciated here.

I think you're right. I remember some discussion with Gunnar about this specific subject when we decided to only keep the group conversion of the type arguments but it's definitely not specified as is in the spec.

Considering it's a corner case that is explicitly not recommended, I think we should simply drop this test and keep the behavior undefined for now and do better in the next revision of the spec.

We decided back then to not mandate any specific behaviour if both the legacy @Valid and the type-argument @Valid are given, but indeed that fact is not totally clear from the spec. I've logged https://hibernate.atlassian.net/browse/BVAL-709 to clarify that this is unspecified (unless someone truly thinks this *should* be specified).

In that light indeed the TCK test should be removed.

Reading again this part, I think the "(in order to prevent the container elements from being validated twice)" part should also go away as it's at the discretion of the implementation to validate them twice or once (especially given we have some infinite loop detection going on to prevent circular dependencies that might intervene and IIRC we also don't want constraints to be enforced twice).

Note that this sentence is not there to state that implementations *should* honour both @Valid. Instead it's there to say that some implementation *may* do it, and to be sure that only one is applied, one should only give @Valid in one place.

> Next, can anyone explain the intent of @SpecAssertion#id() ?

They are used to link the TCK tests to specific sentences/phrases from the spec. If you build the TCK from source, you'll find a file tests/target/coverage-report/coverage-beanvalidation.html which contains all phrases from the spec marked as TCK-relevant, allowing you to see the link between a test method and the related spec sentences.

In that case, though, the id marker seems off, as the referenced one is about Set, whereas this the test is about List (not that it matters, as the test is going away anyways now).

Gunnar is at a conference right now, we'll discuss that more in depth when he's back but I'd say don't worry about this test.

About the TCK tests being ignored, could you please open another thread with more details on your TCK setup (pointers to the code and how you run it would be nice) so we don't pollute this one?

Thanks.

--
Guillaume

_______________________________________________
beanvalidation-dev mailing list
beanvalidation-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/beanvalidation-dev