Jakarta TCKs #33638
Merged
Jakarta TCKs #33638
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
quarkus-bot
bot
added
area/arc
Ladicek
force-pushed
the
jakarta-tcks
branch
4 times, most recently
from
6a03312 to
7a55f18
Compare
|
#33523 was merged and CDI TCK 4.0.10 was released, so this PR is now rebased and ready. |
|
Also adding @geoand for the testing stuff. |
Awesome, almost there now! :) |
This comment has been minimized.
This comment has been minimized.
geoand
reviewed
May 30, 2023
I always love this kind of cleanup! Moving reflection as high up as possible is such an enabler :) |
The Build Compatible Extension API specifies that classes added through `ScannedClasses` during `@Discovery` are always discovered types and therefore always beans. This requires a change in the type discovery implementation in the Quarkus ArC extension: all classes added through `ScannedClasses` are subject to an annotation transformation that adds a bean defining annotation (`@AdditionalBean`).
The Quarkus ArC extension assumes that the root application archive is always a bean archive. That doesn't necessarily have to be true per the CDI specification, but is useful in practice. Therefore, it needs to be fixed only in strict mode, which is what this commit does. By default, the root application archive is always a bean archive. In strict mode, the root application archive goes through the same scanning process as other application archives.
The Quarkus ArC extension used to discover bean archives that only contain qualifiers or interceptor bindings. This commit fixes that to also discover bean archives that only contain scopes.
… TCK This extension was used to activate/deactivate the CDI request context before/after each test. The Quarkus Arquillian adapter contains the exact same extension, so the one in MP Context Propagation TCK is useless.
Arquillian holds onto deployment-scoped (and container-scoped) objects until its end of life. That allows undeploying and redeploying without losing contextual data, but is also technically a memory leak. In case the deployment-scoped objects are large (which is the case in Quarkus, see `RunningQuarkusApplication`), this memory leak becomes very visible very soon. The Quarkus Arquillian adapter is only used to run TCKs for Jakarta and MicroProfile specifications, which don't need this capability. Also, the `RunningQuarkusApplication` is unusable after it is closed, so it makes no sense to keep it around. Hence, `QuarkusDeployableContainer` no longer stores the `RunningQuarkusApplication` into the deployment context directly; instead, it stores a holder object (`QuarkusDeployment`) that can safely be cleaned up after an Arquillian deployment is undeployed. That makes the Quarkus application unreachable and GC-able. Arquillian will keep piling up empty objects, which is still a memory leak, but one that doesn't hurt as much. This commit also fixes an ordering issue. Previously, the Arquillian adapter used to delete the Quarkus application directory _before_ shutting down the application, which could lead to errors. This commit switches the order: the app is shut down first and only then is it deleted.
This commit moves the code that looks up a test class from CDI into a dedicated delegate class. This class is loaded from the Quarkus CL and so the code can be written directly instead of reflectively. Additionally, the CDI lookup can now correctly instantiate a generic test class. Such bean cannot be looked up from CDI directly, because its set of bean types doesn't contain a `Class` object (it contains a `ParameterizedType` instead). This commit uses a naive approach for this (iterate through all beans), which is fine, because it's very uncommon.
- AtInject - CDI (Lite) - CDI Lang Model
|
Issues should be gone now. The MP Context Propagation TCK used to fail because when refactoring the Arquillian adapter, I forgot to update possible consumers -- fortunately, the consumer in MP ConProp TCK was redundant, so I just deleted it. The MP LRA TCK used to fail because when refactoring the Arquillian adapter, I failed to transform all |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
✔️ The latest workflow run for the pull request has completed successfully. It should be safe to merge provided you have a look at the other checks in the summary. |
mkouba
approved these changes
May 31, 2023
manovotn
approved these changes
May 31, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #28558