InvocationContextTest likely incomplete

[struberg]

Hi!

It seems that the Test got totally changed. It now tests something very different than before, but without updating the spec section! In commit 021f446d5bceffc7169ec4636 a lot of similar changes got made:

 -    @Interceptors({ Interceptor8.class, Interceptor9.class })
 +    @SimpleBinding

But this changes the mechanism from using javax.interceptor.Interceptors to CDI style Interceptors. This is something very different!

[manovotn]

Hello

The assertions in the test haven't changed since 2017, the supporting classes indeed did change. Can you please point me to the section of interceptor specification which would explain why the test is now invalid or asserting something unexpected?

[struberg]

This test used to check the CDI support for javax.interceptor.Interceptors. Not it tests support for the @InterceptorBinding mechanism. Thats's just something very different, don't you agree? Of course, we should ideally test both!

[manovotn]

This test used to check the CDI support for javax.interceptor.Interceptors

Did it really? If you take a look at the @SpecAssertion values on the test in question and cross-reference it with TCK audit file, you will see that these tests in fact perform assertions on InvocationContext object. I couldn't find anything in the test that would be specific to usage or functionality of javax.interceptor.Interceptors versus bindings. So no, I do not agree - both variants of the test IMO test the same functionality.

[struberg]

Applying interceptor via @InterceptorBinding is something different than via @jakarta.interceptor.Interceptors. It's a different mechanism. You ASSUME that both behave the same internally. But how can you guarantee that if you removed the test for the later mechanism?

[manovotn]

You ASSUME that both behave the same internally.

That's not true, I simply stated that the test is asserting InvocationContext, not functionality or behavior of either @InterceptorBinding or @jakarta.interceptor.Interceptors - there are different tests for those. And as far as I can tell from the specification, you are allowed to use InvocationContext regardless of what interceptor "style" you chose.

Therefore, my initial question stands:

Can you please point me to the section of interceptor specification which would explain why the test is now invalid or asserting something unexpected?

[struberg]

You are right, it's not invalid or asserting something unexpected. But we only test one out of 2 different routes. We imo should test both.

edit I changed the title of the ticket to better reflect my request. Indeed it's not 'broken' but more 'incomplete'