Further enhancements to core/test-extension and extension-authors-guide #1678
Conversation
|
Great, I see the JDK 11 build is failing due to JAXB being pulled out. |
| <dependency> | ||
| <groupId>org.glassfish.jaxb</groupId> | ||
| <artifactId>jaxb-runtime</artifactId> | ||
| </dependency> |
There was a problem hiding this comment.
I think it should be done in the quarkus-jaxb extension and you should depend on it.
There was a problem hiding this comment.
I would be concerned the core module is depending on too many modules outside of the core?
There was a problem hiding this comment.
what's the resolution here? There's lots of good stuff in this PR, I'd rather merge it than wait.
Is it important we have the quarkus-jaxb module if this jaxb dep is only used for testing? @gsmet could you open a separate issue if you think so? I'll merge soon if I don't see blockers :)
There was a problem hiding this comment.
Right, I would rather get the content out and then fix the dependency issue for the next release once I can verify that it does not introduce the issues like the native image build dependency caused. I created an issue for this. #1819
core/test-extension/runtime/src/main/java/io/quarkus/extest/runtime/subst/KeyProxy.java
Show resolved
Hide resolved
| :numbered: | ||
| :sectnums: | ||
| :sectnumlevels: 4 | ||
| :toc: |
There was a problem hiding this comment.
We will need to check how it looks like on the website.
There was a problem hiding this comment.
We should IMHO add TOC to all guides - recently, I created #1490
core/test-extension/deployment/src/test/java/io/quarkus/extest/ConfiguredBean.java
Outdated
Show resolved
Hide resolved
| public final class TestProcessor { | ||
| static DotName TEST_ANNOTATION = DotName.createSimple(TestAnnotation.class.getName()); | ||
| static DotName TEST_ANNOTATION_SCOPE = DotName.createSimple(ApplicationScoped.class.getName()); |
There was a problem hiding this comment.
You can replace this with io.quarkus.arc.processor.BuiltinScope.APPLICATION.getInfo().getDotName(). Maybe, we should add a convenient method like BuiltinScope.getName(). Also the defaultScope is not mandatory - @Dependent is used by default.
|
The CI build failures should not be resolved as the devtools dependency in the core/test-extension have been moved into the integration/test-extension test module |
starksm64
dismissed
gsmet’s stale review
Review is out of date, see latest changes
|
@starksm64 it looks like some of my comments were not addressed. Could you take a look, please? |
|
Ok, not sure why I keep missing some of your comments, but I have addressed the majority of them. |
|
there's a failure with JDK11: |
|
Strange, this seemed to pass previously. I have removed that import as it was unused. |
|
We've just recently become strict about using non-standard classes, in order to prevent build errors of the flavor previously seen when the project is partially built with Java 9 or later and then a native image is produced (unsuccessfully due to referencing Java 9+ symbols and overloads). |
|
@Sanne should be good to go |
|
thanks @starksm64 . I see you merged master into your feature branch, isn't that confusing? You mind if I rebase it one last time? |
|
No, that is fine. I'm not sure how that happened. |
…s-guide Add a code sample section based on the core test-extension Add a new section that shows various command tasks using the core test-extension code Updates for the Sample Test Extension section of the extension-authors-guide Updates for the Sample Test Extension section of the extension-authors-guide Updates for the Sample Test Extension section of the extension-authors-guide Updates for the Sample Test Extension section of the extension-authors-guide Updates for the Sample Test Extension section of the extension-authors-guide Initial native image test and updates Additional examples Reformat Reformatting Sketch out object substitution and and replacement Fix formatting Rename the native image integration test Update comment Update comments Add sections on deployment scanning and bean container interface Reformat Reformat Don't rely on JDK JAXB so code compiles and runs under both JDK8/11 Updates from review Fix rebase issues Updates for the Sample Test Extension section of the extension-authors-guide Add a code sample section based on the core test-extension Add a new section that shows various command tasks using the core test-extension code Updates for the Sample Test Extension section of the extension-authors-guide Updates for the Sample Test Extension section of the extension-authors-guide Updates for the Sample Test Extension section of the extension-authors-guide Updates for the Sample Test Extension section of the extension-authors-guide Updates for the Sample Test Extension section of the extension-authors-guide Initial native image test and updates Additional examples Reformat Reformatting Sketch out object substitution and and replacement Fix formatting Rename the native image integration test Update comment Update comments Add sections on deployment scanning and bean container interface Updates from review Another rebase attempt Remove undertow-servlet dependency Another rebase attempt Remove the quarkus maven plugin Update for AdditionalBeanBuildItem changes
…hang the integration tests
The PR implements more of the testing outlined in #780 and adds a new Sample Test Extension section to the extension-authors-guide that illustrates several common extension tasks. A TOC was added and the header formatting was corrected to make the document more navigable.