Multiple QuarkusTestResource annotations in Test are ignored

[lkrzyzanek]

Describe the bug
Multiple @QuarkusTestResource annotations in Test are completely ignored
e.g.
@QuarkusTest
@QuarkusTestResource(ActiveMQServerTestResource.class)
@QuarkusTestResource(ActiveMQClientTestResource.class)

Expected behavior
Given that QuarkusTestResource annotation is Repeatable both test resources should be processed.

Actual behavior
Both QuarkusTestResource are ignored

To Reproduce
Steps to reproduce the behavior:

1. Create a test with two simple "QuarkusTestResource" test resources
2. Run test
3. See that both QuarkusTestResource are ignored. If you comment one of them just 1 is picked and processed correctly.

[lkrzyzanek]

The trouble seems to be in method TestResourceManager.getTestResources here: https://github.com/quarkusio/quarkus/blob/master/test-framework/common/src/main/java/io/quarkus/test/common/TestResourceManager.java#L88

When I'm debugging the index variable it contains list of all annotations but in case of multiple QuarkusTestResource annotations the name is io.quarkus.test.common.QuarkusTestResource$List and no io.quarkus.test.common.QuarkusTestResource

[gsmet]

@mkouba could you have a look at that one? There might be something fishy in how Jandex handles repeatable annotations but we really need to figure it out as most of our code is not prepared for having them handled a different way than usual annotations.

[gastaldi]

Definitely a bug. I managed to reproduce it in this testcase:

package io.quarkus.test.common;

import java.util.Collections;
import java.util.Map;
import java.util.concurrent.atomic.AtomicInteger;

import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;

public class TestResourceManagerTest {

    @Test
    void testRepeatableAnnotationsAreIndexed() {
        AtomicInteger counter = new AtomicInteger();
        TestResourceManager manager = new TestResourceManager(MyTest.class);
        manager.inject(counter);
        assertThat(counter.intValue()).isEqualTo(2);
    }


    @QuarkusTestResource(FirstLifecycleManager.class)
    @QuarkusTestResource(SecondLifecycleManager.class)
    public static class MyTest {}


    public static class FirstLifecycleManager implements QuarkusTestResourceLifecycleManager {
        @Override
        public Map<String, String> start() {
            return Collections.emptyMap();
        }

        @Override
        public void inject(Object testInstance) {
            ((AtomicInteger)testInstance).incrementAndGet();
        }

        @Override
        public void stop() {

        }
    }

    public static class SecondLifecycleManager implements QuarkusTestResourceLifecycleManager {
        @Override
        public Map<String, String> start() {
            return Collections.emptyMap();
        }

        @Override
        public void inject(Object testInstance) {
            ((AtomicInteger)testInstance).incrementAndGet();
        }

        @Override
        public void stop() {

        }
    }

}

[mkouba]

the name is io.quarkus.test.common.QuarkusTestResource$List and no io.quarkus.test.common.QuarkusTestResource...

@lkrzyzanek Yes, that's expected - for repeatable annotations you get the "container" annotation.

@gastaldi Thanks for the reproducer!

[gsmet]

@mkouba what bothers me a bit with this is that there's a good chance some of our code does not deal with this properly. I would have expected to get the inner annotations when trying to get them.

[gastaldi]

I have a fix for this, let me send a PR. Feel free to reject or accept it :)

[gsmet]

Is it a general fix or a localized one? I'm really not comfortable having everyone to think about this every time they want to get annotations.

[gastaldi]

A localized one, the general fix would involve changing Jandex AFAIK

[mkouba]

there's a good chance some of our code does not deal with this properly...

There is. On the other hand, any repeatable annotation processed by a build step should have a test. I know we implemented this for io.quarkus.vertx.web.Route and io.quarkus.scheduler.Scheduled.

I would have expected to get the inner annotations when trying to get them.

Well, repeatable annotations are compiled into the container annotations.

We can't change the behavior of IndexView#getAnnotation() as it could break existing code. Reflection API also has specific methods to handle repeatable annotations.

[gastaldi]

Since this issue is assigned to @mkouba (just realized after opening the PR) I closed mine in favor of his 😉