Add scope to beans to allow tests to pass in environments with implic…

[manovotn]

…it bean archives.

I came across this while trying to run FT tests in Quarkus.

Functionally this doesn't change anything. It merely makes the relevant tests pass in a CDI environment that operates on implicit bean archive. In the current master, these classes are automatically picked as beans anyway, the PR only adds a bean defining annotation on them.

There might be more tests needing similar changes but so far I've only seen these.

[manovotn]

This is similar to what Ken described in #454

[Azquelt]

Interestingly, this shouldn't actually need an annotation to cause the test to pass, even if it's not picked up as a bean.

FallbackMethodSubclassBeanA extends this class and validating that bean should be sufficient to cause an exception to be thrown at startup.

Not having the superclass annotated is consistent with other similar tests in the same package (FallbackMethodSubclassOverrideBeanB, FallbackMethodSuperclassPrivateBeanB, etc.)

I'm wondering if it would improve these tests to have the beans.xml specify that only annotated beans should be included.

[manovotn]

FallbackMethodSubclassBeanA extends this class and validating that bean should be sufficient to cause an exception to be thrown at startup.

Then I perhaps misunderstood the test?
The subclass (FallbackMethodSubclassBeanA) seems OK because that bean contains the fallback method and inherits the method annotated with @Fallback.
Whereas FallbackMethodSubclassBeanB, when picked up as a bean as validated fails because it declares a @Fallback method (named fallback), but that method is not declared anywhere in the bean/class hierarchy.
What am I missing @Azquelt ? :)

I'm wondering if it would improve these tests to have the beans.xml specify that only annotated beans should be included.

We might do that as well. That's basically what my and Ken's PR are aimed towards - for the tests to work in an environment with implicit bean archives.

[Azquelt]

You're not allowed to reference a fallback method from a subclass.

We decided on that to try to be consistent with the guidelines in the Common Annotations for the Java Platform spec (section 2.1). Basically, I should be able to look at the @Fallback annotation and know what it's going to do without having to look further around the class hierarchy.

The relevant part of the fault tolerance spec is in 8.1.2:

When fallbackMethod is used a FaultToleranceDefinitionException will be thrown if any of the following constraints are not met:

• The named fallback method must be on the same class, a superclass or an implemented interface of the class which declares the annotated method
  [...]
• The named fallback method must be accessible from the class which declares the annotated method

Note: this doesn't prevent you from referencing a public or protected fallback method which is in the same class and then overriding that method in a subclass.

[manovotn]

I see, thanks for explanation!
In that case, we should probably add @Vetoed to FallbackMethodSubclassBeanB so that it doesn't get picked up and validated accidentally because that leads to the same exception as the one the test is originally supposed to cover.

[Azquelt]

Yeah, I agree, the test doesn't currently test what it's supposed to. Vetoing the class is probably clearer than excluding non-annotated classes in the beans.xml.

[manovotn]

@Azquelt I have amended the PR, it now adds the scope to one test and vetoes the other bean.

[Azquelt]

Thanks :)