[bv-dev] TCK: LegacyValidOnContainerCascadingTest

[Gunnar Morling]

Hi,

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

> Hi Matt,
>
> On Tue, Mar 13, 2018 at 12:18 AM, Matt Benson <mbenson at 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 at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/beanvalidation-dev/attachments/20180319/19a712db/attachment.html