Skip to content

test(config): stop SuiteRunnerIssueTestSample from re-entering itself - #3400

Merged
juherr merged 1 commit into
testng-team:masterfrom
kalayciburak:fix/suite-runner-sample-self-recursion
Aug 24, 2026
Merged

test(config): stop SuiteRunnerIssueTestSample from re-entering itself#3400
juherr merged 1 commit into
testng-team:masterfrom
kalayciburak:fix/suite-runner-sample-self-recursion

Conversation

@kalayciburak

@kalayciburak kalayciburak commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

SuiteRunnerIssueTestSample built a nested XmlSuite whose only XmlClass was itself. Running the sample re-entered suiteRunnerSample unbounded, hit StackOverflowError (sometimes inside XMLStringBuffer's static initializer), and left ConfigurationTest.testSuiteRunnerWithDefaultConfiguration failing with status 1 when run in isolation.

The nested suite now points at a separate trivial sample so the SuiteRunner plus default listeners path from #2743 actually terminates.

Fixes #3360

Test plan

  • ./gradlew :testng-core:test --tests test.configuration.ConfigurationTest.testSuiteRunnerWithDefaultConfiguration — RED (expected: 0 but was: 1) then GREEN (1/1)
  • ./gradlew :testng-core:test --tests test.configuration.ConfigurationTest — 19/19 GREEN
  • ./gradlew autostyleCheck GREEN
  • ./gradlew rewriteDryRun — recipes would make no changes

Summary by CodeRabbit

  • Bug Fixes
    • Improved suite configuration coverage for nested test execution.
    • Updated suite setup to correctly register nested sample tests.
  • Tests
    • Added coverage for nested suite runner scenarios.
    • Linked the relevant test case to issue GITHUB-2743.
    • Added a dedicated nested test sample to validate suite registration and execution.

@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9600a379-fb61-4d9d-a3a0-ec4e93dd1ae6

📥 Commits

Reviewing files that changed from the base of the PR and between 5596ba9 and d2fef87.

📒 Files selected for processing (1)
  • testng-core/src/test/java/test/configuration/ConfigurationTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • testng-core/src/test/java/test/configuration/ConfigurationTest.java

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

The test setup adds a separate nested sample class, registers it in the generated XML suite, and labels the related configuration test with GITHUB-2743.

Changes

SuiteRunner test fix

Layer / File(s) Summary
Register separate nested sample
testng-core/src/test/java/test/configuration/issue2743/SuiteRunnerIssueNestedSample.java, testng-core/src/test/java/test/configuration/issue2743/SuiteRunnerIssueTestSample.java, testng-core/src/test/java/test/configuration/ConfigurationTest.java
The suite uses SuiteRunnerIssueNestedSample.class instead of recursively registering SuiteRunnerIssueTestSample. The configuration test annotation includes the GITHUB-2743 description.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to d2fef

The change prevents the suite-runner sample from recursively invoking itself and restores termination for the affected configuration test path; no actionable merge-blocking risk remains.

Suggested reviewers: krmahadevan

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

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.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the fix for SuiteRunnerIssueTestSample self-reentry.
Linked Issues check ✅ Passed The changes satisfy [#3360] by replacing the self-referential nested suite with a separate sample class, preventing recursion and restoring regression coverage.
Out of Scope Changes check ✅ Passed All changes directly support the recursion fix and the related #2743 regression test; no unrelated code changes are present.
✨ 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

Comment @coderabbitai help to get the list of available commands.

}

@Test
@Test(description = "GITHUB-3360")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@Test(description = "GITHUB-3360")
@Test(description = "GITHUB-2743")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, switched to GITHUB-2743

The nested XmlSuite listed the sample class as its only XmlClass, so
running it rebuilt the same suite unbounded. Isolated runs hit
StackOverflowError (sometimes inside XMLStringBuffer's initializer)
and ConfigurationTest.testSuiteRunnerWithDefaultConfiguration failed
with status 1.

Point the nested suite at a separate trivial sample so the SuiteRunner
plus default listeners path from testng-team#2743 actually terminates.

Fixes testng-team#3360
@kalayciburak
kalayciburak force-pushed the fix/suite-runner-sample-self-recursion branch from 5596ba9 to d2fef87 Compare August 20, 2026 08:56
@juherr
juherr merged commit 6a1bfa1 into testng-team:master Aug 24, 2026
12 checks passed
@juherr juherr added this to the 7.13.0 milestone Sep 7, 2026 — with ChatGPT Codex Connector
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants