fix(QTDI-2284): fix junit resources cleanup by thboileau · Pull Request #1179 · Talend/component-runtime

thboileau · 2026-03-11T16:00:37Z

Requirements

Any code change adding any logic MUST be tested through a unit test executed with the default build
Any API addition MUST be done with a documentation update if relevant

Why this PR is needed?

What does this PR adds (design/code thoughts)?

thboileau · 2026-03-11T16:01:10Z

...ponent-runtime-junit/src/main/java/org/talend/sdk/component/junit/BaseComponentsHandler.java

                .get(ComponentManager.AllServices.class)
                .getServices();
-        Injector.class.cast(services.get(Injector.class)).inject(instance);
+        ((Injector) services.get(Injector.class)).inject(instance);


Intellij asked me to do so

Useless change

thboileau · 2026-03-11T16:02:40Z

...ponent-runtime-junit/src/main/java/org/talend/sdk/component/junit/BaseComponentsHandler.java

    }

-    interface Local<T> {
+    protected interface Local<T> {


Intellij detected that the interface Local is used with a protected variable. Hence it's better to make this class protected as well

protected static final Local<State> STATE = loadStateHolder();

Copilot

Pull request overview

This PR aims to fix JUnit-related resource cleanup in component-runtime-junit, making the embedded component manager shutdown safer (notably when closed multiple times) and adding a regression test.

Changes:

Bump the project’s JUnit 5 version property.
Update BaseComponentsHandler cleanup logic (centralize JSON-B close and avoid NPE on repeated close() calls).
Add a unit test validating that the embedded manager can be closed twice without leaving state behind.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
`pom.xml`	Updates the JUnit 5 version property used across the build.
`component-runtime-testing/component-runtime-junit/src/main/java/org/talend/sdk/component/junit/BaseComponentsHandler.java`	Makes embedded manager cleanup more robust; adds `State.cleanUp()` and adjusts shutdown behavior; minor doc/formatting tweaks.
`component-runtime-testing/component-runtime-junit/src/main/java/org/talend/sdk/component/junit5/ComponentExtension.java`	Minor Javadoc grammar fix.
`component-runtime-testing/component-runtime-junit/src/test/java/org/talend/sdk/component/junit/BaseComponentHandlerTest.java`	Adds regression coverage for double-close behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

...ent-runtime-junit/src/test/java/org/talend/sdk/component/junit/BaseComponentHandlerTest.java

Copilot · 2026-03-11T16:08:18Z

...ent-runtime-junit/src/test/java/org/talend/sdk/component/junit/BaseComponentHandlerTest.java

+public class BaseComponentHandlerTest {
+
+    @Test
+    void canCloseTheEmbeddedManagerTwice() {
+        final MyBaseComponentsHandler myBaseComponentsHandler = new MyBaseComponentsHandler("org.talend.sdk.component.junit");
+        final BaseComponentsHandler.EmbeddedComponentManager start = myBaseComponentsHandler.start();
+
+        assertNotNull(BaseComponentsHandler.STATE.get());
+        start.close();
+        assertNull(BaseComponentsHandler.STATE.get());
+        start.close();
+        assertNull(BaseComponentsHandler.STATE.get());
+    }
+
+    private static class MyBaseComponentsHandler extends BaseComponentsHandler {
+        public MyBaseComponentsHandler(final String packageName) {
+            this.packageName = packageName;
+        }
+    }


Naming: BaseComponentHandlerTest/MyBaseComponentsHandler look like typos/inconsistencies (missing the 's' from BaseComponentsHandler). Renaming the test class and helper to match the production type name would make the intent clearer.

Copilot · 2026-03-11T16:08:18Z

...ponent-runtime-junit/src/main/java/org/talend/sdk/component/junit/BaseComponentsHandler.java

+     * IMPORTANT: don't forget to consume all the streams to ensure the underlying
     * { @see org.talend.sdk.component.runtime.input.Input} is closed.


The Javadoc says callers must "consume all the streams", but this method returns a single Stream. This wording is misleading and should be corrected back to singular (or rephrased) to match the actual API contract.

Suggested change

* IMPORTANT: don't forget to consume all the streams to ensure the underlying

* { @see org.talend.sdk.component.runtime.input.Input} is closed.

* IMPORTANT: don't forget to consume the returned stream to ensure the

* underlying {@link org.talend.sdk.component.runtime.input.Input} is closed.

Copilot · 2026-03-11T16:08:18Z

...ponent-runtime-junit/src/main/java/org/talend/sdk/component/junit/BaseComponentsHandler.java

    }

-    interface Local<T> {
+    protected interface Local<T> {


Local was changed from package-private to protected, which expands the API surface of BaseComponentsHandler to subclasses in other packages. Since there are no in-repo usages of BaseComponentsHandler.Local, consider keeping it package-private unless an external use-case requires this visibility change.

Suggested change

protected interface Local<T> {

interface Local<T> {

...ent-runtime-junit/src/test/java/org/talend/sdk/component/junit/BaseComponentHandlerTest.java

...ponent-runtime-junit/src/main/java/org/talend/sdk/component/junit/BaseComponentsHandler.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

ypiel-talend

1 minor remark

ypiel-talend · 2026-03-12T10:50:38Z

...ponent-runtime-junit/src/main/java/org/talend/sdk/component/junit/BaseComponentsHandler.java

                .get(ComponentManager.AllServices.class)
                .getServices();
-        Injector.class.cast(services.get(Injector.class)).inject(instance);
+        ((Injector) services.get(Injector.class)).inject(instance);


Useless change

thboileau added 2 commits March 11, 2026 15:36

fix(QTDI-2284): fix junit resources cleanup

c4befc3

fix(QTDI-2284): add junit test

56323ee

thboileau commented Mar 11, 2026

View reviewed changes

thboileau changed the title ~~Tboileau/qtdi 2284 junit cleanup issue~~ fix(QTDI-2284): fix junit resources cleanup Mar 11, 2026

thboileau requested review from Copilot and undx March 11, 2026 16:03

Copilot started reviewing on behalf of thboileau March 11, 2026 16:04 View session

Copilot AI reviewed Mar 11, 2026

View reviewed changes

thboileau commented Mar 11, 2026

View reviewed changes

...ponent-runtime-junit/src/main/java/org/talend/sdk/component/junit/BaseComponentsHandler.java Outdated Show resolved Hide resolved

thboileau and others added 3 commits March 11, 2026 17:37

Apply suggestion from @thboileau

96d903f

Apply suggestion from @Copilot

8cde4c2

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Apply suggestions from code review

2bbd210

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

thboileau requested review from ypiel-talend and removed request for undx March 12, 2026 10:41

ypiel-talend approved these changes Mar 12, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(QTDI-2284): fix junit resources cleanup#1179

fix(QTDI-2284): fix junit resources cleanup#1179
thboileau wants to merge 5 commits intomasterfrom
tboileau/QTDI-2284_junit_cleanup_issue

thboileau commented Mar 11, 2026

Uh oh!

thboileau Mar 11, 2026

Uh oh!

ypiel-talend Mar 12, 2026

Uh oh!

thboileau Mar 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Uh oh!

Uh oh!

ypiel-talend left a comment

Uh oh!

ypiel-talend Mar 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		* IMPORTANT: don't forget to consume all the streams to ensure the underlying
		* { @see org.talend.sdk.component.runtime.input.Input} is closed.

Conversation

thboileau commented Mar 11, 2026

Requirements

Why this PR is needed?

What does this PR adds (design/code thoughts)?

Uh oh!

thboileau Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

ypiel-talend Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

thboileau Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ypiel-talend left a comment

Choose a reason for hiding this comment

Uh oh!

ypiel-talend Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants