Task/bugfix 2578

[krmahadevan]

Fixes #2578.

Did you remember to?

• Add test case(s)
• Update CHANGES.txt
• Auto applied styling via ./gradlew autostyleApply

We encourage pull requests that:

• Add new features to TestNG (or)
• Fix bugs in TestNG

If your pull request involves fixing SonarQube issues then we would suggest that you please discuss this with the
TestNG-dev before you spend time working on it.

Note: For more information on contribution guidelines please make sure you refer our Contributing section for detailed set of steps.

Summary by CodeRabbit

• Bug Fixes

  • Improved handling of listeners with unavailable constructor dependencies, ensuring TestNG continues execution without throwing exceptions when a listener cannot be instantiated due to missing dependencies.

• Tests

  • Added regression test for listener instantiation with missing constructor dependencies.

[coderabbitai]

📝 Walkthrough

Walkthrough

The InstanceCreator now uses MethodHandles to invoke no-arg constructors instead of reflection-based approach, with updated exception handling. A short-circuit optimization delegates empty-parameter calls to the zero-arg path. New regression tests verify listeners with unavailable constructor dependencies don't cause TestNG failures.

Changes

Cohort / File(s)	Summary
Constructor Invocation Refactoring testng-core-api/src/main/java/org/testng/internal/objects/InstanceCreator.java	Replaces getDeclaredConstructor().newInstance() with MethodHandles for no-arg constructor invocation. Changes exception handling to wrap reflection failures into TestNGException via two catch blocks (IllegalAccessException | NoSuchMethodException | SecurityException and broader Throwable). Adds empty-parameters short-circuit to delegate to zero-arg path.
Regression Test for Issue #2578 testng-core/src/test/java/test/listeners/ListenersTest.java	Adds ensureListenerWithUnavailableConstructorDependencyDoesNotFail() test that verifies TestNG completes successfully when a listener's constructor dependency is unavailable (loaded via custom classloader raising NoClassDefFoundError with ClassNotFoundException cause). Integrates AssertJ helpers for assertions.
Test Fixtures for Issue #2578 testng-core/src/test/java/test/listeners/issue2578/ListenerWithMissingConstructorDependency.java, testng-core/src/test/java/test/listeners/issue2578/MissingConstructorDependency.java, testng-core/src/test/java/test/listeners/issue2578/MissingDependencyClassLoader.java, testng-core/src/test/java/test/listeners/issue2578/TestClassSample.java	Provides test infrastructure: ListenerWithMissingConstructorDependency (implements ITestListener with dual constructors), MissingConstructorDependency (unavailable dependency), MissingDependencyClassLoader (custom loader that throws ClassNotFoundException for specific class), and TestClassSample (annotated test class using the listener).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

• juherr

Poem

🐰 A hop through handles, smooth and clean,
where constructors dance unseen.
Missing friends don't break the test—
our TestNG now handles the rest!
No more silent crashes in the night,
the listeners load just right! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'Task/bugfix 2578' is vague and non-descriptive, using a task reference without explaining what the actual change accomplishes.	Use a more descriptive title that explains the change, such as 'Fix instance creation to properly handle missing constructor dependencies' or 'Improve error handling in InstanceCreator for missing dependencies'.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	The PR successfully implements the core requirement: it changes InstanceCreator to handle instantiation failures more gracefully by using MethodHandles and wrapping exceptions into TestNGException, preventing cascading NoClassDefFoundError failures.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing issue #2578: the InstanceCreator refactoring addresses the root cause, and the test files (listener class, custom class loader, test fixtures) form a coherent regression test.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

testng-core-api/src/main/java/org/testng/internal/objects/InstanceCreator.java (1)

28-37: Redundant catch clauses can be merged.

The IllegalAccessException | NoSuchMethodException | SecurityException branch (lines 32–33) and the Throwable branch (lines 34–36) build the exact same TestNGException with the same message and cause, so the first clause is fully subsumed by the second. Collapsing them removes duplication without changing behavior.

♻️ Proposed simplification

   public static <T> T newInstance(Class<T> clazz) {
     try {
       MethodHandle constructor =
           MethodHandles.lookup().findConstructor(clazz, MethodType.methodType(void.class));
       return (T) constructor.invoke();
-    } catch (IllegalAccessException | NoSuchMethodException | SecurityException e) {
-      throw new TestNGException(CANNOT_INSTANTIATE_CLASS + clazz.getName(), e);
     } catch (Throwable e) {
       throw new TestNGException(CANNOT_INSTANTIATE_CLASS + clazz.getName(), e);
     }
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@testng-core-api/src/main/java/org/testng/internal/objects/InstanceCreator.java`
around lines 28 - 37, The two catch blocks that both throw new
TestNGException(CANNOT_INSTANTIATE_CLASS + clazz.getName(), e) are redundant;
merge them into a single catch that catches Throwable (or the narrower common
supertype you prefer) and rethrows the TestNGException, keeping the existing
message and cause so behavior is unchanged—update the try/catch around
MethodHandles.lookup().findConstructor(clazz, MethodType.methodType(void.class))
/ constructor.invoke() to use a single catch referencing the same
TestNGException and CANNOT_INSTANTIATE_CLASS + clazz.getName().

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@testng-core-api/src/main/java/org/testng/internal/objects/InstanceCreator.java`:
- Around line 28-37: The two catch blocks that both throw new
TestNGException(CANNOT_INSTANTIATE_CLASS + clazz.getName(), e) are redundant;
merge them into a single catch that catches Throwable (or the narrower common
supertype you prefer) and rethrows the TestNGException, keeping the existing
message and cause so behavior is unchanged—update the try/catch around
MethodHandles.lookup().findConstructor(clazz, MethodType.methodType(void.class))
/ constructor.invoke() to use a single catch referencing the same
TestNGException and CANNOT_INSTANTIATE_CLASS + clazz.getName().

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 16096982-4580-422a-b89a-34a861fb15ed

📥 Commits

Reviewing files that changed from the base of the PR and between 88c32ff and 0c0c1b6.

📒 Files selected for processing (6)

• testng-core-api/src/main/java/org/testng/internal/objects/InstanceCreator.java
• testng-core/src/test/java/test/listeners/ListenersTest.java
• testng-core/src/test/java/test/listeners/issue2578/ListenerWithMissingConstructorDependency.java
• testng-core/src/test/java/test/listeners/issue2578/MissingConstructorDependency.java
• testng-core/src/test/java/test/listeners/issue2578/MissingDependencyClassLoader.java
• testng-core/src/test/java/test/listeners/issue2578/TestClassSample.java