DROOLS-7503 simplify testing of the Quarkus BuildItems #5386

tarilabs · 2023-07-11T14:19:00Z

Simplify testing of the BuildItems produced by the drools-drl-quarkus extension, avoiding the need for a "fake" downstream extension just to test the content of the builditems.

See summary of previous strategy in: quarkusio/quarkus#32389

As suggested by @geoand this PR instead uses QuarkusUnitTest#addBuildChainCustomizer. It's like adding a synthetic ~"build step" for the test phase; and I'm now using that, in place of the concrete buildsteps in the "fake" downstream test extension.

Inspired by test found by call of this methods here in the quarkus repo: https://github.com/quarkusio/quarkus/blob/80826ecd747ac056847100e480ad778a9f6bc888/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusUnitTest.java#L251

JIRA: https://issues.redhat.com/browse/DROOLS-7503

referenced Pull Requests: none

How to replicate CI configuration locally?

Build Chain tool does "simple" maven build(s), the builds are just Maven commands, but because the repositories relates and depends on each other and any change in API or class method could affect several of those repositories there is a need to use build-chain tool to handle cross repository builds and be sure that we always use latest version of the code for each repository.

build-chain tool is a build tool which can be used on command line locally or in Github Actions workflow(s), in case you need to change multiple repositories and send multiple dependent pull requests related with a change you can easily reproduce the same build by executing it on Github hosted environment or locally in your development environment. See local execution details to get more information about it.

How to retest this PR or trigger a specific build:

for pull request checks
Please add comment: Jenkins retest this
for a specific pull request check
Please add comment: Jenkins (re)run [drools|kogito-runtimes|kogito-apps|kogito-examples] tests
for a full downstream build
- for jenkins job: please add comment: Jenkins run fdb
- for github actions job: add the label run_fdb
a compile downstream build please add comment: Jenkins run cdb
a full production downstream build please add comment: Jenkins execute product fdb
an upstream build please add comment: Jenkins run upstream
for quarkus branch checks
Run checks against Quarkus current used branch
Please add comment: Jenkins run quarkus-branch
for a quarkus branch specific check
Run checks against Quarkus current used branch
Please add comment: Jenkins (re)run [drools|kogito-runtimes|kogito-apps|kogito-examples] quarkus-branch
for quarkus main checks
Run checks against Quarkus main branch
Please add comment: Jenkins run quarkus-main
for a specific quarkus main check
Run checks against Quarkus main branch
Please add comment: Jenkins (re)run [drools|kogito-runtimes|kogito-apps|kogito-examples] quarkus-main
for quarkus lts checks
Run checks against Quarkus lts branch
Please add comment: Jenkins run quarkus-lts
for a specific quarkus lts check
Run checks against Quarkus lts branch
Please add comment: Jenkins (re)run [drools|kogito-runtimes|kogito-apps|kogito-examples] quarkus-lts
for native checks
Run native checks
Please add comment: Jenkins run native
for a specific native check
Run native checks
Please add comment: Jenkins (re)run [drools|kogito-runtimes|kogito-apps|kogito-examples] native
for native lts checks
Run native checks against quarkus lts branch
Please add comment: Jenkins run native-lts
for a specific native lts check
Run native checks against quarkus lts branch
Please add comment: Jenkins (re)run [drools|kogito-runtimes|kogito-apps|kogito-examples] native-lts

How to backport a pull request to a different branch?

In order to automatically create a backporting pull request please add one or more labels having the following format backport-<branch-name>, where <branch-name> is the name of the branch where the pull request must be backported to (e.g., backport-7.67.x to backport the original PR to the 7.67.x branch).

NOTE: backporting is an action aiming to move a change (usually a commit) from a branch (usually the main one) to another one, which is generally referring to a still maintained release branch. Keeping it simple: it is about to move a specific change or a set of them from one branch to another.

Once the original pull request is successfully merged, the automated action will create one backporting pull request per each label (with the previous format) that has been added.

If something goes wrong, the author will be notified and at this point a manual backporting is needed.

NOTE: this automated backporting is triggered whenever a pull request on main branch is labeled or closed, but both conditions must be satisfied to get the new PR created.

tarilabs · 2023-07-11T14:20:28Z

...s-drl-quarkus-deployment/src/test/java/org/drools/drl/quarkus/deployment/BuildItemsTest.java

+            .addBuildChainCustomizer(b -> {
+                b.addBuildStep(new BuildStep() {
+                    @Override
+                    public void execute(BuildContext context) {
+                        GlobalsBuildItem gbi = context.consume(GlobalsBuildItem.class);
+                        assertGlobals(gbi.getGlobals());
+
+                        CombinedIndexBuildItem indexBI = context.consume(CombinedIndexBuildItem.class);
+                        PatternsTypesBuildItem otnClasesBI = context.consume(PatternsTypesBuildItem.class);
+                        Set<String> allKnown = computeAllKnown(otnClasesBI, indexBI);
+                        assertAllKnownOTNs(allKnown);
+                    }
+                })


@geoand could you kindly confirm this is a good pattern to test the resulting builditems generated by this very extensions? I'm using assertions on those BIs.

tarilabs · 2023-07-11T14:21:02Z

drools-drl-quarkus-extension/pom.xml

-        <module>example-drools-drl-quarkus-testext</module>
-        <module>example-drools-drl-quarkus-testext-deployment</module>


avoid the need of "fake" downstream extensions only to test this extension.

tarilabs · 2023-07-11T14:21:48Z

...testext/src/main/java/org/drools/drl/quarkus/testext/deployment/GlobalsMetadataRecorder.java

-    public BeanContainerListener setContent(Map<String, Map<String, java.lang.reflect.Type>> contentValue) {
-        return beanContainer -> {
-            GlobalsSingleton bean = beanContainer.beanInstance(GlobalsSingleton.class);
-            bean.setGlobals(contentValue);
-        };


also avoid the need of bytecoderecorder just to capture the values of the builditems to be used in the tests

dupliaka · 2023-07-12T08:18:53Z

...s-drl-quarkus-deployment/src/test/java/org/drools/drl/quarkus/deployment/BuildItemsTest.java

+
+    @Test
+    public void testBean() {
+        // find assertions in addBuildChainCustomizer


As i get it you create a bean with configuration for Quarkus extension.
For testing you create an example application and build chain for that so it builds the BI and so you can assert it.

How about renaming the method name?
Also it creates a Code smell that could be avoided by adding the assert
assertThatNoException();

cool with me, but what do I put in the body?

assertThatNoException().isThrownBy(() -> System.out.println("OK"));

this method does not have to do nothing

about the method name, https://github.com/kiegroup/drools/pull/5386/files#r1260805157

Could be it - instead of OK you can put the message that quarkus extension build chain successfully built

Suggested change

// find assertions in addBuildChainCustomizer

assertThatNoException().isThrownBy(() -> LOG.info("looking for a successfully executed QuarkusUnitTest with custom .addBuildChainCustomizer() containing assertions"));

done with beb5b33

tarilabs · 2023-07-12T08:31:34Z

...s-drl-quarkus-deployment/src/test/java/org/drools/drl/quarkus/deployment/BuildItemsTest.java

+            });
+
+    @Test
+    public void testBean() {


Suggested change

public void testBean() {

public void test() {

I am afraid the next commit would be t(); :) I meant to expand the purpose of the test in the name

Suggested change

public void testBean() {

public void testQuarkusUTByAddBuildChainCustomizer() {

done with beb5b33

pibizza

Minor comments on tests, since I am not that familiar with this piece of code. Looks good to me otherwise.

pibizza · 2023-07-12T08:49:35Z

...s-drl-quarkus-deployment/src/test/java/org/drools/drl/quarkus/deployment/BuildItemsTest.java

+                MyImplementation.class.getCanonicalName(),
+                MyInterface.class.getCanonicalName())
+                .as("these classes or subclasses are derived from OTNs in the rules.");
+


This does not work. You have to put the as() clause just after the assertThat.

"This does not work." -> it won't actually show the specified message string. well spotted, thanks

done with aaf7789

pibizza · 2023-07-12T08:49:44Z

...s-drl-quarkus-deployment/src/test/java/org/drools/drl/quarkus/deployment/BuildItemsTest.java

+
+        assertThat(allKnown).doesNotContain(MyUnusedClass.class.getCanonicalName())
+                .as("this class is unused in the rules despite star-import");
+    }


This does not work. You have to put the as() clause just after the assertThat.

"This does not work." -> it won't actually show the specified message string. well spotted, thanks

done with aaf7789

sonarcloud · 2023-07-13T11:27:15Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

DROOLS-7503 simplify testing of the Quarkus BuildItems

d6577f7

tarilabs requested review from danielezonca, mariofusco, lucamolteni, tkobayas, pibizza and dupliaka July 11, 2023 14:19

tarilabs commented Jul 11, 2023

View reviewed changes

tkobayas approved these changes Jul 12, 2023

View reviewed changes

mariofusco approved these changes Jul 12, 2023

View reviewed changes

dupliaka reviewed Jul 12, 2023

View reviewed changes

tarilabs commented Jul 12, 2023

View reviewed changes

dupliaka approved these changes Jul 12, 2023

View reviewed changes

code review: implement feedback

beb5b33

pibizza approved these changes Jul 13, 2023

View reviewed changes

code review feedback

aaf7789

lucamolteni approved these changes Jul 13, 2023

View reviewed changes

danielezonca approved these changes Jul 17, 2023

View reviewed changes

danielezonca merged commit 1c5bec9 into apache:main Jul 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DROOLS-7503 simplify testing of the Quarkus BuildItems #5386

DROOLS-7503 simplify testing of the Quarkus BuildItems #5386

tarilabs commented Jul 11, 2023

tarilabs Jul 11, 2023

geoand Jul 11, 2023

tarilabs Jul 11, 2023

tarilabs Jul 11, 2023

dupliaka Jul 12, 2023

tarilabs Jul 12, 2023

tarilabs Jul 12, 2023

dupliaka Jul 12, 2023

tarilabs Jul 12, 2023 •

edited

Loading

tarilabs Jul 12, 2023

tarilabs Jul 12, 2023

dupliaka Jul 12, 2023

tarilabs Jul 12, 2023

tarilabs Jul 12, 2023

pibizza left a comment

pibizza Jul 12, 2023

tarilabs Jul 13, 2023

tarilabs Jul 13, 2023

pibizza Jul 12, 2023

tarilabs Jul 13, 2023 •

edited

Loading

tarilabs Jul 13, 2023

sonarcloud bot commented Jul 13, 2023

		<module>example-drools-drl-quarkus-testext</module>
		<module>example-drools-drl-quarkus-testext-deployment</module>

	// find assertions in addBuildChainCustomizer
	assertThatNoException().isThrownBy(() -> LOG.info("looking for a successfully executed QuarkusUnitTest with custom .addBuildChainCustomizer() containing assertions"));

	public void testBean() {
	public void testQuarkusUTByAddBuildChainCustomizer() {

DROOLS-7503 simplify testing of the Quarkus BuildItems #5386

DROOLS-7503 simplify testing of the Quarkus BuildItems #5386

Conversation

tarilabs commented Jul 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tarilabs Jul 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pibizza left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tarilabs Jul 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Jul 13, 2023

tarilabs Jul 12, 2023 •

edited

Loading

tarilabs Jul 13, 2023 •

edited

Loading