jakartaee / platform-tck

Jakartaee-tck
Other
128 stars 109 forks source link

Core profile CDI tests use archive without beans.xml #1526

Open arjantijms opened 1 month ago

arjantijms commented 1 month ago

The test ee.jakarta.tck.core.rest.jsonb.cdi.CustomJsonbSerializationIT in the 10.0.3 core profile TCK creates the following archive:

test archive: CustomJsonbSerializationIT.war:
/WEB-INF/
/WEB-INF/classes/
/WEB-INF/classes/ee/
/WEB-INF/classes/ee/jakarta/
/WEB-INF/classes/ee/jakarta/tck/
/WEB-INF/classes/ee/jakarta/tck/core/
/WEB-INF/classes/ee/jakarta/tck/core/rest/
/WEB-INF/classes/ee/jakarta/tck/core/rest/jsonb/
/WEB-INF/classes/ee/jakarta/tck/core/rest/jsonb/cdi/
/WEB-INF/classes/ee/jakarta/tck/core/rest/jsonb/cdi/ApplicationResource.class
/WEB-INF/classes/ee/jakarta/tck/core/rest/jsonb/cdi/CustomJsonbResolver.class
/WEB-INF/classes/ee/jakarta/tck/core/rest/jsonb/cdi/CustomJsonbResolver$CustomSerializer.class
/WEB-INF/classes/ee/jakarta/tck/core/rest/jsonb/cdi/CustomJsonbResolver$CustomDeserializer.class
/WEB-INF/classes/ee/jakarta/tck/core/rest/jsonb/cdi/KeysProducer.class
/WEB-INF/classes/ee/jakarta/tck/core/rest/jsonb/cdi/SomeMessage.class
/WEB-INF/classes/ee/jakarta/tck/core/rest/jsonb/cdi/Utils.class
/WEB-INF/classes/ee/jakarta/tck/core/rest/JaxRsActivator.class
/WEB-INF/classes/key.pub

According to https://jakarta.ee/specifications/cdi/4.0/jakarta-cdi-spec-4.0#bean_archive the beans.xml file is required.

How are we supposed to pass this, or did I interpret something incorrectly? A number of products have already passed the Core TCK, so I'm not entirely sure what I'm missing, if anything.

manovotn commented 1 month ago

How are we supposed to pass this, or did I interpret something incorrectly? A number of products have already passed the Core TCK, so I'm not entirely sure what I'm missing, if anything.

I don't know the precise reason why and how they passed but EE implementations can mark deployment as CDI ones (meaning they will trigger Weld startup) based on various other indicators than just beans.xml presence. But it is just a guess.

Either way, there should IMO be beans.xml in the test so long as you are supposed to pass this with CDI Lite.

arjantijms commented 1 month ago

How are we supposed to pass this?

I'd say add beans.xml to the test. Is that a problem?

@manovotn & @Ladicek You mean the vendor has to do that, or that we update the test to contain beans.xml?

For now I've added the following to the test runner, but I guess you meant the latter option?

public class CustomJsonbSerializationITFix implements LoadableExtension {

    @Override
    public void register(ExtensionBuilder builder) {
        builder.observer(CustomJsonbSerializationITFix.class);
    }

    public void removeService(@Observes BeforeDeploy event) {
        Archive<?> archive = event.getDeployment().getArchive();
        if (archive instanceof WebArchive webArchive) {
            if (webArchive.getName().equals("CustomJsonbSerializationIT.war")) {
                webArchive.addAsWebInfResource(EmptyAsset.INSTANCE, "beans.xml");
            }
        }
    }

}
manovotn commented 1 month ago

You mean the vendor has to do that, or that we update the test to contain beans.xml?

I mean I'd add beans.xml to the TCK test in this repo - I think this is a valid challenge. Your workaround is also a viable option in the meantime :+1:

jhanders34 commented 1 week ago

Would https://github.com/jakartaee/platform-tck/blob/main/core-profile-tck/tck/src/main/java/ee/jakarta/tck/core/rest/context/app/ApplicationContextIT.java#L47 also need to be updated to include a beans.xml? That REST resource has an @Inject in it as well?

starksm64 commented 1 week ago

Would https://github.com/jakartaee/platform-tck/blob/main/core-profile-tck/tck/src/main/java/ee/jakarta/tck/core/rest/context/app/ApplicationContextIT.java#L47 also need to be updated to include a beans.xml? That REST resource has an @Inject in it as well?

Yes, good point, every archive without an explicit beans.xml should have an empty one.

jhanders34 commented 1 week ago

To be clear, this is a Core Profile 10.x challenge. I assume the fixes done in the PRs associated with this issue also need to be ported to the Core Profile TCK version 10 branch and a 10.x service release created.

starksm64 commented 1 week ago

Right, I'm just focused on the EE11 TCK currently.