You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by "ruthst00 (via GitHub)" <gi...@apache.org> on 2026/09/22 18:52:07 UTC
[PR] Fixed issue where transaction sample elapsed time was incorrectly inf… [jmeter]
ruthst00 opened a new pull request, #6770:
URL: https://github.com/apache/jmeter/pull/6770
…lated by timer/pause delays when a thread was stopped mid-transaction (e.g., during ramp-down).
## Description
<!--- Provide a general summary of your changes in the Title above -->
<!--- Describe your changes in detail here -->
### Root Cause
__Parent mode__ (`Generate parent sample = true`): When a thread was stopped mid-transaction (e.g., scheduler end time reached), `JMeterThread.processSampler()` called `doEndTransactionSampler()` directly without first calling `TransactionSampler.setTransactionDone()`. This meant the transaction result's end time and idle time were never properly computed — the elapsed time was set by `addSubResult()` to `lastChildEndTime - startTime`, which included any timer delays between child samples.
__Non-parent mode__ (`Generate parent sample = false`): `TransactionController.triggerEndOfLoop()` (called during error/loop-restart scenarios) called `res.sampleEnd()` without first accounting for the time elapsed since the last child sample ended. This time (which could include a timer delay) was incorrectly included in the transaction elapsed time.
### Changes
1. __`TransactionSampler.java`__: Changed `setTransactionDone()` from `protected` to `public` so it can be called from `JMeterThread` (different package).
2. __`JMeterThread.java`__: In the `!running` path of `processSampler()`, call `transactionSampler.setTransactionDone()` before `doEndTransactionSampler()` if not already done. This ensures the idle time and elapsed time are correctly computed when a thread is stopped mid-transaction.
3. __`TransactionController.java`__: In `triggerEndOfLoop()` for non-parent mode, add the time elapsed since the last child sample ended to `pauseTime` (as idle time) before calling `res.sampleEnd()`. This mirrors the same logic already present in `nextWithoutTransactionSampler()`.
## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
Transaction sample elapsed time was incorrectly inflated by timer/pause delays when a thread was stopped mid-transaction (e.g., during ramp-down).
Fixes [GitHub issue #6496](https://github.com/apache/jmeter/issues/6496)
## How Has This Been Tested?
<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, tests ran to see how -->
<!--- your change affects other areas of the code, etc. -->
Added two regression tests to `TestTransactionController.java`:
- `testIssue6496NonParentMode`: Verifies that in non-parent mode, the transaction elapsed time equals the sum of child sample times (not inflated by a timer delay between samples).
- `testIssue6496ParentMode`: Verifies that in parent mode, when the thread is stopped mid-transaction via the scheduler, the transaction elapsed time is not inflated by the long timer delay.
## Screenshots (if appropriate):
## Types of changes
<!--- What types of changes does your code introduce? Delete as appropriate -->
- Bug fix (non-breaking change which fixes an issue)
## Checklist:
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
- [X] My code follows the [code style][style-guide] of this project.
- [ ] I have updated the documentation accordingly.
Generated with [Claude Sonnet](https://www.anthropic.com/claude/sonnet) via [Cline](https://cline.bot/)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixed issue where transaction sample elapsed time was incorrectly inflated when a thread was stopped [jmeter]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on code in PR #6770:
URL: https://github.com/apache/jmeter/pull/6770#discussion_r4106457041
##########
src/components/src/test/java/org/apache/jmeter/control/TestTransactionController.java:
##########
@@ -18,23 +18,224 @@
package org.apache.jmeter.control;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import org.apache.jmeter.assertions.ResponseAssertion;
import org.apache.jmeter.junit.JMeterTestCase;
import org.apache.jmeter.sampler.DebugSampler;
+import org.apache.jmeter.samplers.AbstractSampler;
+import org.apache.jmeter.samplers.Entry;
+import org.apache.jmeter.samplers.SampleResult;
import org.apache.jmeter.test.samplers.CollectSamplesListener;
+import org.apache.jmeter.testelement.AbstractTestElement;
import org.apache.jmeter.threads.JMeterContextService;
import org.apache.jmeter.threads.JMeterThread;
import org.apache.jmeter.threads.JMeterVariables;
import org.apache.jmeter.threads.ListenerNotifier;
import org.apache.jmeter.threads.TestCompiler;
import org.apache.jmeter.threads.ThreadGroup;
+import org.apache.jmeter.timers.Timer;
import org.apache.jorphan.collections.ListedHashTree;
import org.junit.jupiter.api.Test;
public class TestTransactionController extends JMeterTestCase {
+ /**
+ * A simple sampler that returns a successful result with a fixed elapsed time.
+ */
+ private static class FixedElapsedTimeSampler extends AbstractSampler {
Review Comment:
Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixed issue where transaction sample elapsed time was incorrectly inflated when a thread was stopped [jmeter]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on code in PR #6770:
URL: https://github.com/apache/jmeter/pull/6770#discussion_r4106391603
##########
src/components/src/test/java/org/apache/jmeter/control/TestTransactionController.java:
##########
@@ -18,23 +18,224 @@
package org.apache.jmeter.control;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import org.apache.jmeter.assertions.ResponseAssertion;
import org.apache.jmeter.junit.JMeterTestCase;
import org.apache.jmeter.sampler.DebugSampler;
+import org.apache.jmeter.samplers.AbstractSampler;
+import org.apache.jmeter.samplers.Entry;
+import org.apache.jmeter.samplers.SampleResult;
import org.apache.jmeter.test.samplers.CollectSamplesListener;
+import org.apache.jmeter.testelement.AbstractTestElement;
import org.apache.jmeter.threads.JMeterContextService;
import org.apache.jmeter.threads.JMeterThread;
import org.apache.jmeter.threads.JMeterVariables;
import org.apache.jmeter.threads.ListenerNotifier;
import org.apache.jmeter.threads.TestCompiler;
import org.apache.jmeter.threads.ThreadGroup;
+import org.apache.jmeter.timers.Timer;
import org.apache.jorphan.collections.ListedHashTree;
import org.junit.jupiter.api.Test;
public class TestTransactionController extends JMeterTestCase {
+ /**
+ * A simple sampler that returns a successful result with a fixed elapsed time.
+ */
+ private static class FixedElapsedTimeSampler extends AbstractSampler {
+ private static final long serialVersionUID = 1L;
+ private final long elapsedTimeMs;
+
+ FixedElapsedTimeSampler(long elapsedTimeMs) {
+ this.elapsedTimeMs = elapsedTimeMs;
+ }
+
+ @Override
+ public SampleResult sample(Entry e) {
+ SampleResult result = new SampleResult();
+ result.setSampleLabel(getName());
+ result.sampleStart();
+ result.setSuccessful(true);
+ result.setResponseCodeOK();
+ // Simulate elapsed time by setting start/end times directly
+ long start = result.getStartTime();
+ result.setEndTime(start + elapsedTimeMs);
+ return result;
+ }
+ }
+
+ /**
+ * A timer that returns a fixed delay.
+ */
+ private static class FixedDelayTimer extends AbstractTestElement implements Timer {
+ private static final long serialVersionUID = 1L;
+ private final long delayMs;
+
+ FixedDelayTimer(long delayMs) {
+ this.delayMs = delayMs;
+ }
+
+ @Override
+ public long delay() {
+ return delayMs;
+ }
+ }
+
+ /**
+ * Test for GitHub issue #6496: transaction elapsed time should not include timer delay
+ * in non-parent mode (TransactionController with "Generate parent sample" unchecked).
+ *
+ * <p>The test sets up a transaction with two samplers and a timer between them.
+ * The transaction elapsed time should equal the sum of child sample elapsed times,
+ * not be inflated by the timer delay between samples.
+ */
+ @Test
+ public void testIssue6496NonParentMode() throws Exception {
+ JMeterContextService.getContext().setVariables(new JMeterVariables());
+
+ CollectSamplesListener listener = new CollectSamplesListener();
+
+ TransactionController transactionController = new TransactionController();
+ transactionController.setGenerateParentSample(false);
+ transactionController.setIncludeTimers(false);
+
+ // Use a simulated elapsed time much smaller than the timer delay
+ long childElapsedMs = 10L;
+ long timerDelayMs = 200L; // timer delay before second sampler
+
+ FixedElapsedTimeSampler firstSampler = new FixedElapsedTimeSampler(childElapsedMs);
+ firstSampler.setName("First Sampler");
+
+ FixedDelayTimer timer = new FixedDelayTimer(timerDelayMs);
+ timer.setName("Timer Before Second Sampler");
+ timer.setEnabled(true);
+
+ FixedElapsedTimeSampler secondSampler = new FixedElapsedTimeSampler(childElapsedMs);
+ secondSampler.setName("Second Sampler");
+
+ LoopController loop = new LoopController();
+ loop.setLoops(1);
+ loop.setContinueForever(false);
+ loop.setEnabled(true);
+
+ ListedHashTree hashTree = new ListedHashTree();
+ hashTree.add(loop);
+ hashTree.add(loop, transactionController);
+ // In non-parent mode, the TransactionController fires events using the SamplePackage
+ // of the last sampler that ran. The listener must be a child of that sampler.
+ hashTree.add(transactionController, firstSampler);
+ hashTree.add(transactionController, secondSampler);
+ hashTree.add(secondSampler, timer);
+ hashTree.add(secondSampler, listener);
+
+ TestCompiler compiler = new TestCompiler(hashTree);
+ hashTree.traverse(compiler);
+
+ ThreadGroup threadGroup = new ThreadGroup();
+ threadGroup.setNumThreads(1);
+
+ ListenerNotifier notifier = new ListenerNotifier();
+
+ JMeterThread thread = new JMeterThread(hashTree, threadGroup, notifier);
+ thread.setThreadGroup(threadGroup);
+ thread.run();
+
+ assertFalse(listener.getEvents().isEmpty(),
+ "At least one transaction sample should have been collected");
+
+ // Find the transaction result (not the individual sample results)
+ SampleResult transactionResult = listener.getEvents().stream()
+ .map(e -> e.getResult())
+ .filter(TransactionController::isFromTransactionController)
+ .findFirst()
+ .orElse(null);
+ assertTrue(transactionResult != null,
+ "A transaction result should have been collected");
+ // The transaction elapsed time should be the sum of child sample times,
+ // not inflated by the timer delay between samples.
+ // Both child samples have very short simulated elapsed times (10ms each),
+ // so the total should be well under the timer delay (200ms).
+ assertTrue(transactionResult.getTime() < timerDelayMs,
Review Comment:
Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixed issue where transaction sample elapsed time was incorrectly inflated when a thread was stopped [jmeter]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on PR #6770:
URL: https://github.com/apache/jmeter/pull/6770#issuecomment-5835820423
@vlsi here is a checklist of every change you requested, and whether it was addressed in the pushed commit (`cfb87d1f7`):
✅ __Tests capture values inside `sampleOccurred`__ — Done. Replaced `CollectSamplesListener` with a new `SnapshotListener` that records `time`, `idleTime`, and `responseMessage` immediately inside `sampleOccurred()`.
✅ __HashTree construction fixed__ — Done. All tests now use the subtree-chaining form (`tree.add(loop).add(tc)`).
✅ __Tests fail on base commit__ — Verified. Running the tests against `ae8f85e` (base) gives 2 failures; all 3 pass with the fix.
✅ __Drop `.github/workflows/gradle-wrapper-validation.yml`__ — Done. That commit was dropped via interactive rebase.
✅ __Commit message__ — Done. Imperative subject under 72 chars, details in body.
✅ __`xdocs/changes.xml` entry__ — Done. Entry added under Bug fixes / General.
⚠️ __`triggerEndOfLoop()` test__ — The `testIssue6496NonParentMode` test exercises the scheduler-stop path, which does call `triggerEndOfLoop()` indirectly. However, you specifically asked for a test that uses a Flow Control Action "Start next thread loop" (i.e. `continueOnCurrentLoop`/`continueOnThreadLoop`) to trigger `triggerEndOfLoop()` directly, or alternatively to move the `triggerEndOfLoop()` change to a separate PR. The current test does not use a Flow Control Action — it relies on the scheduler stop path instead. This item is __partially addressed__ but may not satisfy your specific request.
⚠️ __`changes.xml` names both the elapsed-time fix AND the `isFromTransactionController()` side-effect__ — you asked for both to be named. The current entry only mentions the elapsed-time fix; the side-effect about `isFromTransactionController()` returning `true` for interrupted transactions (affecting `Summariser`, `ResultSaver`, `SamplerMetric`) was removed to shorten the entry.
⚠️ __State intended behavior for interrupted transactions (success vs. failure)__ — you asked to state in the PR description whether interrupted transactions should be marked as failed or successful, and why. This requires a comment on the PR itself.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixed issue where transaction sample elapsed time was incorrectly inf… [jmeter]
Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on code in PR #6770:
URL: https://github.com/apache/jmeter/pull/6770#discussion_r4098289854
##########
src/core/src/main/java/org/apache/jmeter/threads/JMeterThread.java:
##########
@@ -526,6 +526,12 @@ private SampleResult processSampler(Sampler current, Sampler parent, JMeterConte
&& transactionResult == null
&& transactionSampler != null
&& transactionPack != null) {
+ // Thread was stopped mid-transaction (e.g. during ramp-down).
Review Comment:
- Bug 55816 is about non-parent mode (the time after the last child sample); it has nothing to do with this parent-mode path. Please drop that reference.
- "Ensure setTransactionDone() is called" narrates the next line. Please state the rule instead, for example: "A transaction interrupted by a thread stop reports the sum of its children as elapsed time; the time since the last child ended counts as idle time."
- The issue number should not carry the meaning; the sentence has to state the fact without it.
##########
src/core/src/main/java/org/apache/jmeter/control/TransactionController.java:
##########
@@ -252,6 +252,14 @@ public static boolean isFromTransactionController(SampleResult res) {
public void triggerEndOfLoop() {
if(!isGenerateParentSample()) {
if (res != null) {
+ // See BUG 55816 / GitHub issue #6496
Review Comment:
This comment is incorrect: `triggerEndOfLoop()` is not called when the thread stops. It is called from `JMeterThread.continueOnCurrentLoop`, `breakOnCurrentLoop`, and `continueOnThreadLoop`, that is, when a failed sample triggers "Start Next Thread Loop" or a Flow Control Action ends the loop. In non-parent mode, a thread stop emits no transaction sample at all.
Please describe the actual case, for example: "The loop ends early, so the time since the last child sample ended counts as idle time, as it does in `nextWithoutTransactionSampler()`." Please also cover this path with a test (see the review summary).
`nextWithoutTransactionSampler()` now has the same four lines. Please extract them into a private method so the two paths cannot drift apart.
##########
src/core/src/main/java/org/apache/jmeter/threads/JMeterThread.java:
##########
@@ -526,6 +526,12 @@ private SampleResult processSampler(Sampler current, Sampler parent, JMeterConte
&& transactionResult == null
&& transactionSampler != null
&& transactionPack != null) {
+ // Thread was stopped mid-transaction (e.g. during ramp-down).
+ // Ensure setTransactionDone() is called so that elapsed time and idle time
+ // are correctly computed (see GitHub issue #6496 / Bug 55816).
+ if (!transactionSampler.isTransactionDone()) {
Review Comment:
This check is always true here: this branch runs only when `transactionResult == null`, and a transaction that was already done set `transactionResult` in the first branch of this method, unless `doEndTransactionSampler` threw. Either drop the check, or keep it and say which case it guards.
##########
src/components/src/test/java/org/apache/jmeter/control/TestTransactionController.java:
##########
@@ -18,23 +18,224 @@
package org.apache.jmeter.control;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import org.apache.jmeter.assertions.ResponseAssertion;
import org.apache.jmeter.junit.JMeterTestCase;
import org.apache.jmeter.sampler.DebugSampler;
+import org.apache.jmeter.samplers.AbstractSampler;
+import org.apache.jmeter.samplers.Entry;
+import org.apache.jmeter.samplers.SampleResult;
import org.apache.jmeter.test.samplers.CollectSamplesListener;
+import org.apache.jmeter.testelement.AbstractTestElement;
import org.apache.jmeter.threads.JMeterContextService;
import org.apache.jmeter.threads.JMeterThread;
import org.apache.jmeter.threads.JMeterVariables;
import org.apache.jmeter.threads.ListenerNotifier;
import org.apache.jmeter.threads.TestCompiler;
import org.apache.jmeter.threads.ThreadGroup;
+import org.apache.jmeter.timers.Timer;
import org.apache.jorphan.collections.ListedHashTree;
import org.junit.jupiter.api.Test;
public class TestTransactionController extends JMeterTestCase {
+ /**
+ * A simple sampler that returns a successful result with a fixed elapsed time.
+ */
+ private static class FixedElapsedTimeSampler extends AbstractSampler {
Review Comment:
`setEndTime(start + elapsedTimeMs)` sets an end time in the future without waiting for it. The child then appears to finish after the transaction's `currentTimeInMillis()`, and `TransactionSampler.setTransactionDone()` can compute a negative idle time. Either make the fixed elapsed time explicit with `setStampAndTime(start, elapsedTimeMs)` and assert the resulting idle time too, or add a comment that the test relies on this.
##########
src/core/src/main/java/org/apache/jmeter/control/TransactionSampler.java:
##########
@@ -119,7 +119,7 @@ public void addSubSamplerResult(SampleResult res) {
totalConnectTime += res.getConnectTime();
}
- protected void setTransactionDone() {
+ public void setTransactionDone() {
Review Comment:
This makes `setTransactionDone()` public API, while calling it at the wrong moment finalizes a transaction that is still running. Please mark it `@API(status = API.Status.INTERNAL, since = "6.0.0")` (apiguardian is already used in `src/core`), or add a narrower public method that `JMeterThread` calls to end an interrupted transaction.
A subclass that overrides this method as `protected` stops compiling after this change; please mention that in `changes.xml` under incompatible changes, or avoid widening this method.
##########
src/components/src/test/java/org/apache/jmeter/control/TestTransactionController.java:
##########
@@ -18,23 +18,224 @@
package org.apache.jmeter.control;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import org.apache.jmeter.assertions.ResponseAssertion;
import org.apache.jmeter.junit.JMeterTestCase;
import org.apache.jmeter.sampler.DebugSampler;
+import org.apache.jmeter.samplers.AbstractSampler;
+import org.apache.jmeter.samplers.Entry;
+import org.apache.jmeter.samplers.SampleResult;
import org.apache.jmeter.test.samplers.CollectSamplesListener;
+import org.apache.jmeter.testelement.AbstractTestElement;
import org.apache.jmeter.threads.JMeterContextService;
import org.apache.jmeter.threads.JMeterThread;
import org.apache.jmeter.threads.JMeterVariables;
import org.apache.jmeter.threads.ListenerNotifier;
import org.apache.jmeter.threads.TestCompiler;
import org.apache.jmeter.threads.ThreadGroup;
+import org.apache.jmeter.timers.Timer;
import org.apache.jorphan.collections.ListedHashTree;
import org.junit.jupiter.api.Test;
public class TestTransactionController extends JMeterTestCase {
+ /**
+ * A simple sampler that returns a successful result with a fixed elapsed time.
+ */
+ private static class FixedElapsedTimeSampler extends AbstractSampler {
+ private static final long serialVersionUID = 1L;
+ private final long elapsedTimeMs;
+
+ FixedElapsedTimeSampler(long elapsedTimeMs) {
+ this.elapsedTimeMs = elapsedTimeMs;
+ }
+
+ @Override
+ public SampleResult sample(Entry e) {
+ SampleResult result = new SampleResult();
+ result.setSampleLabel(getName());
+ result.sampleStart();
+ result.setSuccessful(true);
+ result.setResponseCodeOK();
+ // Simulate elapsed time by setting start/end times directly
+ long start = result.getStartTime();
+ result.setEndTime(start + elapsedTimeMs);
+ return result;
+ }
+ }
+
+ /**
+ * A timer that returns a fixed delay.
+ */
+ private static class FixedDelayTimer extends AbstractTestElement implements Timer {
+ private static final long serialVersionUID = 1L;
+ private final long delayMs;
+
+ FixedDelayTimer(long delayMs) {
+ this.delayMs = delayMs;
+ }
+
+ @Override
+ public long delay() {
+ return delayMs;
+ }
+ }
+
+ /**
+ * Test for GitHub issue #6496: transaction elapsed time should not include timer delay
+ * in non-parent mode (TransactionController with "Generate parent sample" unchecked).
+ *
+ * <p>The test sets up a transaction with two samplers and a timer between them.
+ * The transaction elapsed time should equal the sum of child sample elapsed times,
+ * not be inflated by the timer delay between samples.
+ */
+ @Test
+ public void testIssue6496NonParentMode() throws Exception {
Review Comment:
This test passes on the base commit: with loop count 1 and no scheduler, the transaction ends through the regular end-of-controller path in `nextWithoutTransactionSampler()`, which already excluded timer delays (Bug 55816). It does not reach the changed `triggerEndOfLoop()`.
Please rename the test after the behavior rather than the ticket, for example `aTransactionEndedByStartNextLoopExcludesTimerDelayFromElapsedTime`, and drive it through the changed path (a failed child with "Start Next Thread Loop", or a Flow Control Action).
##########
src/components/src/test/java/org/apache/jmeter/control/TestTransactionController.java:
##########
@@ -18,23 +18,224 @@
package org.apache.jmeter.control;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import org.apache.jmeter.assertions.ResponseAssertion;
import org.apache.jmeter.junit.JMeterTestCase;
import org.apache.jmeter.sampler.DebugSampler;
+import org.apache.jmeter.samplers.AbstractSampler;
+import org.apache.jmeter.samplers.Entry;
+import org.apache.jmeter.samplers.SampleResult;
import org.apache.jmeter.test.samplers.CollectSamplesListener;
+import org.apache.jmeter.testelement.AbstractTestElement;
import org.apache.jmeter.threads.JMeterContextService;
import org.apache.jmeter.threads.JMeterThread;
import org.apache.jmeter.threads.JMeterVariables;
import org.apache.jmeter.threads.ListenerNotifier;
import org.apache.jmeter.threads.TestCompiler;
import org.apache.jmeter.threads.ThreadGroup;
+import org.apache.jmeter.timers.Timer;
import org.apache.jorphan.collections.ListedHashTree;
import org.junit.jupiter.api.Test;
public class TestTransactionController extends JMeterTestCase {
+ /**
+ * A simple sampler that returns a successful result with a fixed elapsed time.
+ */
+ private static class FixedElapsedTimeSampler extends AbstractSampler {
+ private static final long serialVersionUID = 1L;
+ private final long elapsedTimeMs;
+
+ FixedElapsedTimeSampler(long elapsedTimeMs) {
+ this.elapsedTimeMs = elapsedTimeMs;
+ }
+
+ @Override
+ public SampleResult sample(Entry e) {
+ SampleResult result = new SampleResult();
+ result.setSampleLabel(getName());
+ result.sampleStart();
+ result.setSuccessful(true);
+ result.setResponseCodeOK();
+ // Simulate elapsed time by setting start/end times directly
+ long start = result.getStartTime();
+ result.setEndTime(start + elapsedTimeMs);
+ return result;
+ }
+ }
+
+ /**
+ * A timer that returns a fixed delay.
+ */
+ private static class FixedDelayTimer extends AbstractTestElement implements Timer {
+ private static final long serialVersionUID = 1L;
+ private final long delayMs;
+
+ FixedDelayTimer(long delayMs) {
+ this.delayMs = delayMs;
+ }
+
+ @Override
+ public long delay() {
+ return delayMs;
+ }
+ }
+
+ /**
+ * Test for GitHub issue #6496: transaction elapsed time should not include timer delay
+ * in non-parent mode (TransactionController with "Generate parent sample" unchecked).
+ *
+ * <p>The test sets up a transaction with two samplers and a timer between them.
+ * The transaction elapsed time should equal the sum of child sample elapsed times,
+ * not be inflated by the timer delay between samples.
+ */
+ @Test
+ public void testIssue6496NonParentMode() throws Exception {
+ JMeterContextService.getContext().setVariables(new JMeterVariables());
+
+ CollectSamplesListener listener = new CollectSamplesListener();
+
+ TransactionController transactionController = new TransactionController();
+ transactionController.setGenerateParentSample(false);
+ transactionController.setIncludeTimers(false);
+
+ // Use a simulated elapsed time much smaller than the timer delay
+ long childElapsedMs = 10L;
+ long timerDelayMs = 200L; // timer delay before second sampler
+
+ FixedElapsedTimeSampler firstSampler = new FixedElapsedTimeSampler(childElapsedMs);
+ firstSampler.setName("First Sampler");
+
+ FixedDelayTimer timer = new FixedDelayTimer(timerDelayMs);
+ timer.setName("Timer Before Second Sampler");
+ timer.setEnabled(true);
+
+ FixedElapsedTimeSampler secondSampler = new FixedElapsedTimeSampler(childElapsedMs);
+ secondSampler.setName("Second Sampler");
+
+ LoopController loop = new LoopController();
+ loop.setLoops(1);
+ loop.setContinueForever(false);
+ loop.setEnabled(true);
+
+ ListedHashTree hashTree = new ListedHashTree();
+ hashTree.add(loop);
+ hashTree.add(loop, transactionController);
+ // In non-parent mode, the TransactionController fires events using the SamplePackage
+ // of the last sampler that ran. The listener must be a child of that sampler.
+ hashTree.add(transactionController, firstSampler);
+ hashTree.add(transactionController, secondSampler);
+ hashTree.add(secondSampler, timer);
+ hashTree.add(secondSampler, listener);
+
+ TestCompiler compiler = new TestCompiler(hashTree);
+ hashTree.traverse(compiler);
+
+ ThreadGroup threadGroup = new ThreadGroup();
+ threadGroup.setNumThreads(1);
+
+ ListenerNotifier notifier = new ListenerNotifier();
+
+ JMeterThread thread = new JMeterThread(hashTree, threadGroup, notifier);
+ thread.setThreadGroup(threadGroup);
+ thread.run();
+
+ assertFalse(listener.getEvents().isEmpty(),
+ "At least one transaction sample should have been collected");
+
+ // Find the transaction result (not the individual sample results)
+ SampleResult transactionResult = listener.getEvents().stream()
+ .map(e -> e.getResult())
+ .filter(TransactionController::isFromTransactionController)
+ .findFirst()
+ .orElse(null);
+ assertTrue(transactionResult != null,
+ "A transaction result should have been collected");
+ // The transaction elapsed time should be the sum of child sample times,
+ // not inflated by the timer delay between samples.
+ // Both child samples have very short simulated elapsed times (10ms each),
+ // so the total should be well under the timer delay (200ms).
+ assertTrue(transactionResult.getTime() < timerDelayMs,
+ "Transaction elapsed time (" + transactionResult.getTime() + " ms) should not include " +
+ "the timer delay (" + timerDelayMs + " ms)");
+ }
+
+ /**
+ * Test for GitHub issue #6496: transaction elapsed time should not include timer delay
+ * when the thread is stopped mid-transaction (e.g. during ramp-down), in parent mode
+ * (TransactionController with "Generate parent sample" checked).
+ *
+ * <p>The test uses a scheduler end time that expires before the timer delay completes,
+ * simulating a thread being stopped mid-transaction during ramp-down. The transaction
+ * elapsed time should equal the child sample elapsed time, not be inflated by the timer.
+ */
+ @Test
+ public void testIssue6496ParentMode() throws Exception {
Review Comment:
This test passes on the base commit and does not reproduce the scenario from #6496. The 5000 ms timer runs before the only sampler, `TimerService.adjustDelay` returns `-1` at once, and `running` becomes `false` before any child runs. The transaction has zero children and an elapsed time of 0 with or without the fix.
The bug needs at least one child that ran after a delay, and the stop has to happen on the delay before the next child: for example a 500 ms timer on the first sampler, a 5000 ms timer on the second one, and `setEndTime(now + 1500)`. The expected elapsed time is then the first child's time, and the idle time holds the 500 ms delay.
Please also rename the test after the behavior, for example `aTransactionInterruptedByTheSchedulerExcludesTimerDelayFromElapsedTime`.
##########
src/components/src/test/java/org/apache/jmeter/control/TestTransactionController.java:
##########
@@ -18,23 +18,224 @@
package org.apache.jmeter.control;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import org.apache.jmeter.assertions.ResponseAssertion;
import org.apache.jmeter.junit.JMeterTestCase;
import org.apache.jmeter.sampler.DebugSampler;
+import org.apache.jmeter.samplers.AbstractSampler;
+import org.apache.jmeter.samplers.Entry;
+import org.apache.jmeter.samplers.SampleResult;
import org.apache.jmeter.test.samplers.CollectSamplesListener;
+import org.apache.jmeter.testelement.AbstractTestElement;
import org.apache.jmeter.threads.JMeterContextService;
import org.apache.jmeter.threads.JMeterThread;
import org.apache.jmeter.threads.JMeterVariables;
import org.apache.jmeter.threads.ListenerNotifier;
import org.apache.jmeter.threads.TestCompiler;
import org.apache.jmeter.threads.ThreadGroup;
+import org.apache.jmeter.timers.Timer;
import org.apache.jorphan.collections.ListedHashTree;
import org.junit.jupiter.api.Test;
public class TestTransactionController extends JMeterTestCase {
+ /**
+ * A simple sampler that returns a successful result with a fixed elapsed time.
+ */
+ private static class FixedElapsedTimeSampler extends AbstractSampler {
+ private static final long serialVersionUID = 1L;
+ private final long elapsedTimeMs;
+
+ FixedElapsedTimeSampler(long elapsedTimeMs) {
+ this.elapsedTimeMs = elapsedTimeMs;
+ }
+
+ @Override
+ public SampleResult sample(Entry e) {
+ SampleResult result = new SampleResult();
+ result.setSampleLabel(getName());
+ result.sampleStart();
+ result.setSuccessful(true);
+ result.setResponseCodeOK();
+ // Simulate elapsed time by setting start/end times directly
+ long start = result.getStartTime();
+ result.setEndTime(start + elapsedTimeMs);
+ return result;
+ }
+ }
+
+ /**
+ * A timer that returns a fixed delay.
+ */
+ private static class FixedDelayTimer extends AbstractTestElement implements Timer {
+ private static final long serialVersionUID = 1L;
+ private final long delayMs;
+
+ FixedDelayTimer(long delayMs) {
+ this.delayMs = delayMs;
+ }
+
+ @Override
+ public long delay() {
+ return delayMs;
+ }
+ }
+
+ /**
+ * Test for GitHub issue #6496: transaction elapsed time should not include timer delay
Review Comment:
Please open the Javadoc with the rule the test guards, then state the old defect in one past-tense sentence, for example: "A transaction whose thread stops before the next child sample reports the sum of its children as elapsed time. The timer delays before its children used to be counted in the elapsed time (#6496)." "Test for GitHub issue #6496" says nothing once the number is covered.
##########
src/components/src/test/java/org/apache/jmeter/control/TestTransactionController.java:
##########
@@ -18,23 +18,224 @@
package org.apache.jmeter.control;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import org.apache.jmeter.assertions.ResponseAssertion;
import org.apache.jmeter.junit.JMeterTestCase;
import org.apache.jmeter.sampler.DebugSampler;
+import org.apache.jmeter.samplers.AbstractSampler;
+import org.apache.jmeter.samplers.Entry;
+import org.apache.jmeter.samplers.SampleResult;
import org.apache.jmeter.test.samplers.CollectSamplesListener;
+import org.apache.jmeter.testelement.AbstractTestElement;
import org.apache.jmeter.threads.JMeterContextService;
import org.apache.jmeter.threads.JMeterThread;
import org.apache.jmeter.threads.JMeterVariables;
import org.apache.jmeter.threads.ListenerNotifier;
import org.apache.jmeter.threads.TestCompiler;
import org.apache.jmeter.threads.ThreadGroup;
+import org.apache.jmeter.timers.Timer;
import org.apache.jorphan.collections.ListedHashTree;
import org.junit.jupiter.api.Test;
public class TestTransactionController extends JMeterTestCase {
+ /**
+ * A simple sampler that returns a successful result with a fixed elapsed time.
+ */
+ private static class FixedElapsedTimeSampler extends AbstractSampler {
+ private static final long serialVersionUID = 1L;
+ private final long elapsedTimeMs;
+
+ FixedElapsedTimeSampler(long elapsedTimeMs) {
+ this.elapsedTimeMs = elapsedTimeMs;
+ }
+
+ @Override
+ public SampleResult sample(Entry e) {
+ SampleResult result = new SampleResult();
+ result.setSampleLabel(getName());
+ result.sampleStart();
+ result.setSuccessful(true);
+ result.setResponseCodeOK();
+ // Simulate elapsed time by setting start/end times directly
+ long start = result.getStartTime();
+ result.setEndTime(start + elapsedTimeMs);
+ return result;
+ }
+ }
+
+ /**
+ * A timer that returns a fixed delay.
+ */
+ private static class FixedDelayTimer extends AbstractTestElement implements Timer {
+ private static final long serialVersionUID = 1L;
+ private final long delayMs;
+
+ FixedDelayTimer(long delayMs) {
+ this.delayMs = delayMs;
+ }
+
+ @Override
+ public long delay() {
+ return delayMs;
+ }
+ }
+
+ /**
+ * Test for GitHub issue #6496: transaction elapsed time should not include timer delay
+ * in non-parent mode (TransactionController with "Generate parent sample" unchecked).
+ *
+ * <p>The test sets up a transaction with two samplers and a timer between them.
+ * The transaction elapsed time should equal the sum of child sample elapsed times,
+ * not be inflated by the timer delay between samples.
+ */
+ @Test
+ public void testIssue6496NonParentMode() throws Exception {
+ JMeterContextService.getContext().setVariables(new JMeterVariables());
+
+ CollectSamplesListener listener = new CollectSamplesListener();
+
+ TransactionController transactionController = new TransactionController();
+ transactionController.setGenerateParentSample(false);
+ transactionController.setIncludeTimers(false);
+
+ // Use a simulated elapsed time much smaller than the timer delay
+ long childElapsedMs = 10L;
+ long timerDelayMs = 200L; // timer delay before second sampler
+
+ FixedElapsedTimeSampler firstSampler = new FixedElapsedTimeSampler(childElapsedMs);
+ firstSampler.setName("First Sampler");
+
+ FixedDelayTimer timer = new FixedDelayTimer(timerDelayMs);
+ timer.setName("Timer Before Second Sampler");
+ timer.setEnabled(true);
+
+ FixedElapsedTimeSampler secondSampler = new FixedElapsedTimeSampler(childElapsedMs);
+ secondSampler.setName("Second Sampler");
+
+ LoopController loop = new LoopController();
+ loop.setLoops(1);
+ loop.setContinueForever(false);
+ loop.setEnabled(true);
+
+ ListedHashTree hashTree = new ListedHashTree();
+ hashTree.add(loop);
+ hashTree.add(loop, transactionController);
+ // In non-parent mode, the TransactionController fires events using the SamplePackage
+ // of the last sampler that ran. The listener must be a child of that sampler.
+ hashTree.add(transactionController, firstSampler);
+ hashTree.add(transactionController, secondSampler);
+ hashTree.add(secondSampler, timer);
+ hashTree.add(secondSampler, listener);
+
+ TestCompiler compiler = new TestCompiler(hashTree);
+ hashTree.traverse(compiler);
+
+ ThreadGroup threadGroup = new ThreadGroup();
+ threadGroup.setNumThreads(1);
+
+ ListenerNotifier notifier = new ListenerNotifier();
+
+ JMeterThread thread = new JMeterThread(hashTree, threadGroup, notifier);
+ thread.setThreadGroup(threadGroup);
+ thread.run();
+
+ assertFalse(listener.getEvents().isEmpty(),
+ "At least one transaction sample should have been collected");
+
+ // Find the transaction result (not the individual sample results)
+ SampleResult transactionResult = listener.getEvents().stream()
+ .map(e -> e.getResult())
+ .filter(TransactionController::isFromTransactionController)
+ .findFirst()
+ .orElse(null);
+ assertTrue(transactionResult != null,
Review Comment:
`assertTrue(x != null)` prints `expected: <true> but was: <false>`. Please use `assertNotNull`, or better, drop the `orElse(null)` and let `orElseThrow()` fail with a message that says which events were collected.
##########
src/components/src/test/java/org/apache/jmeter/control/TestTransactionController.java:
##########
@@ -18,23 +18,224 @@
package org.apache.jmeter.control;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import org.apache.jmeter.assertions.ResponseAssertion;
import org.apache.jmeter.junit.JMeterTestCase;
import org.apache.jmeter.sampler.DebugSampler;
+import org.apache.jmeter.samplers.AbstractSampler;
+import org.apache.jmeter.samplers.Entry;
+import org.apache.jmeter.samplers.SampleResult;
import org.apache.jmeter.test.samplers.CollectSamplesListener;
+import org.apache.jmeter.testelement.AbstractTestElement;
import org.apache.jmeter.threads.JMeterContextService;
import org.apache.jmeter.threads.JMeterThread;
import org.apache.jmeter.threads.JMeterVariables;
import org.apache.jmeter.threads.ListenerNotifier;
import org.apache.jmeter.threads.TestCompiler;
import org.apache.jmeter.threads.ThreadGroup;
+import org.apache.jmeter.timers.Timer;
import org.apache.jorphan.collections.ListedHashTree;
import org.junit.jupiter.api.Test;
public class TestTransactionController extends JMeterTestCase {
+ /**
+ * A simple sampler that returns a successful result with a fixed elapsed time.
+ */
+ private static class FixedElapsedTimeSampler extends AbstractSampler {
+ private static final long serialVersionUID = 1L;
+ private final long elapsedTimeMs;
+
+ FixedElapsedTimeSampler(long elapsedTimeMs) {
+ this.elapsedTimeMs = elapsedTimeMs;
+ }
+
+ @Override
+ public SampleResult sample(Entry e) {
+ SampleResult result = new SampleResult();
+ result.setSampleLabel(getName());
+ result.sampleStart();
+ result.setSuccessful(true);
+ result.setResponseCodeOK();
+ // Simulate elapsed time by setting start/end times directly
+ long start = result.getStartTime();
+ result.setEndTime(start + elapsedTimeMs);
+ return result;
+ }
+ }
+
+ /**
+ * A timer that returns a fixed delay.
+ */
+ private static class FixedDelayTimer extends AbstractTestElement implements Timer {
+ private static final long serialVersionUID = 1L;
+ private final long delayMs;
+
+ FixedDelayTimer(long delayMs) {
+ this.delayMs = delayMs;
+ }
+
+ @Override
+ public long delay() {
+ return delayMs;
+ }
+ }
+
+ /**
+ * Test for GitHub issue #6496: transaction elapsed time should not include timer delay
+ * in non-parent mode (TransactionController with "Generate parent sample" unchecked).
+ *
+ * <p>The test sets up a transaction with two samplers and a timer between them.
+ * The transaction elapsed time should equal the sum of child sample elapsed times,
+ * not be inflated by the timer delay between samples.
+ */
+ @Test
+ public void testIssue6496NonParentMode() throws Exception {
+ JMeterContextService.getContext().setVariables(new JMeterVariables());
+
+ CollectSamplesListener listener = new CollectSamplesListener();
+
+ TransactionController transactionController = new TransactionController();
+ transactionController.setGenerateParentSample(false);
+ transactionController.setIncludeTimers(false);
+
+ // Use a simulated elapsed time much smaller than the timer delay
+ long childElapsedMs = 10L;
+ long timerDelayMs = 200L; // timer delay before second sampler
+
+ FixedElapsedTimeSampler firstSampler = new FixedElapsedTimeSampler(childElapsedMs);
+ firstSampler.setName("First Sampler");
+
+ FixedDelayTimer timer = new FixedDelayTimer(timerDelayMs);
+ timer.setName("Timer Before Second Sampler");
+ timer.setEnabled(true);
+
+ FixedElapsedTimeSampler secondSampler = new FixedElapsedTimeSampler(childElapsedMs);
+ secondSampler.setName("Second Sampler");
+
+ LoopController loop = new LoopController();
+ loop.setLoops(1);
+ loop.setContinueForever(false);
+ loop.setEnabled(true);
+
+ ListedHashTree hashTree = new ListedHashTree();
+ hashTree.add(loop);
+ hashTree.add(loop, transactionController);
+ // In non-parent mode, the TransactionController fires events using the SamplePackage
+ // of the last sampler that ran. The listener must be a child of that sampler.
Review Comment:
The listener does not need to be a child of the sampler. What happens here is that `hashTree.add(secondSampler, timer)` (`add(key).add(value)`) adds `secondSampler` as a new top-level node, and the timer and listener hang under that copy rather than under the sampler inside the Transaction Controller. Please build the tree through the returned subtrees: `HashTree tcTree = hashTree.add(loop).add(transactionController); tcTree.add(firstSampler); tcTree.add(secondSampler).add(timer); tcTree.add(listener);`, and drop this comment.
##########
.github/workflows/gradle-wrapper-validation.yml:
##########
@@ -7,4 +7,4 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v6
- - uses: gradle/actions/wrapper-validation@0723195856401067f7a2779048b490ace7a47d7c # v5.0.2
+ - uses: gradle/actions/wrapper-validation@3f131e8634966bd73d06cc69884922b02e6faf92 # v6.2.0
Review Comment:
Unrelated to #6496. Please move this to its own PR.
##########
src/components/src/test/java/org/apache/jmeter/control/TestTransactionController.java:
##########
@@ -18,23 +18,224 @@
package org.apache.jmeter.control;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import org.apache.jmeter.assertions.ResponseAssertion;
import org.apache.jmeter.junit.JMeterTestCase;
import org.apache.jmeter.sampler.DebugSampler;
+import org.apache.jmeter.samplers.AbstractSampler;
+import org.apache.jmeter.samplers.Entry;
+import org.apache.jmeter.samplers.SampleResult;
import org.apache.jmeter.test.samplers.CollectSamplesListener;
+import org.apache.jmeter.testelement.AbstractTestElement;
import org.apache.jmeter.threads.JMeterContextService;
import org.apache.jmeter.threads.JMeterThread;
import org.apache.jmeter.threads.JMeterVariables;
import org.apache.jmeter.threads.ListenerNotifier;
import org.apache.jmeter.threads.TestCompiler;
import org.apache.jmeter.threads.ThreadGroup;
+import org.apache.jmeter.timers.Timer;
import org.apache.jorphan.collections.ListedHashTree;
import org.junit.jupiter.api.Test;
public class TestTransactionController extends JMeterTestCase {
+ /**
+ * A simple sampler that returns a successful result with a fixed elapsed time.
+ */
+ private static class FixedElapsedTimeSampler extends AbstractSampler {
+ private static final long serialVersionUID = 1L;
+ private final long elapsedTimeMs;
+
+ FixedElapsedTimeSampler(long elapsedTimeMs) {
+ this.elapsedTimeMs = elapsedTimeMs;
+ }
+
+ @Override
+ public SampleResult sample(Entry e) {
+ SampleResult result = new SampleResult();
+ result.setSampleLabel(getName());
+ result.sampleStart();
+ result.setSuccessful(true);
+ result.setResponseCodeOK();
+ // Simulate elapsed time by setting start/end times directly
+ long start = result.getStartTime();
+ result.setEndTime(start + elapsedTimeMs);
+ return result;
+ }
+ }
+
+ /**
+ * A timer that returns a fixed delay.
+ */
+ private static class FixedDelayTimer extends AbstractTestElement implements Timer {
+ private static final long serialVersionUID = 1L;
+ private final long delayMs;
+
+ FixedDelayTimer(long delayMs) {
+ this.delayMs = delayMs;
+ }
+
+ @Override
+ public long delay() {
+ return delayMs;
+ }
+ }
+
+ /**
+ * Test for GitHub issue #6496: transaction elapsed time should not include timer delay
+ * in non-parent mode (TransactionController with "Generate parent sample" unchecked).
+ *
+ * <p>The test sets up a transaction with two samplers and a timer between them.
+ * The transaction elapsed time should equal the sum of child sample elapsed times,
+ * not be inflated by the timer delay between samples.
+ */
+ @Test
+ public void testIssue6496NonParentMode() throws Exception {
+ JMeterContextService.getContext().setVariables(new JMeterVariables());
+
+ CollectSamplesListener listener = new CollectSamplesListener();
+
+ TransactionController transactionController = new TransactionController();
+ transactionController.setGenerateParentSample(false);
+ transactionController.setIncludeTimers(false);
+
+ // Use a simulated elapsed time much smaller than the timer delay
+ long childElapsedMs = 10L;
+ long timerDelayMs = 200L; // timer delay before second sampler
+
+ FixedElapsedTimeSampler firstSampler = new FixedElapsedTimeSampler(childElapsedMs);
+ firstSampler.setName("First Sampler");
+
+ FixedDelayTimer timer = new FixedDelayTimer(timerDelayMs);
+ timer.setName("Timer Before Second Sampler");
+ timer.setEnabled(true);
+
+ FixedElapsedTimeSampler secondSampler = new FixedElapsedTimeSampler(childElapsedMs);
+ secondSampler.setName("Second Sampler");
+
+ LoopController loop = new LoopController();
+ loop.setLoops(1);
+ loop.setContinueForever(false);
+ loop.setEnabled(true);
+
+ ListedHashTree hashTree = new ListedHashTree();
+ hashTree.add(loop);
+ hashTree.add(loop, transactionController);
+ // In non-parent mode, the TransactionController fires events using the SamplePackage
+ // of the last sampler that ran. The listener must be a child of that sampler.
+ hashTree.add(transactionController, firstSampler);
+ hashTree.add(transactionController, secondSampler);
+ hashTree.add(secondSampler, timer);
+ hashTree.add(secondSampler, listener);
+
+ TestCompiler compiler = new TestCompiler(hashTree);
+ hashTree.traverse(compiler);
+
+ ThreadGroup threadGroup = new ThreadGroup();
+ threadGroup.setNumThreads(1);
+
+ ListenerNotifier notifier = new ListenerNotifier();
+
+ JMeterThread thread = new JMeterThread(hashTree, threadGroup, notifier);
+ thread.setThreadGroup(threadGroup);
+ thread.run();
+
+ assertFalse(listener.getEvents().isEmpty(),
+ "At least one transaction sample should have been collected");
+
+ // Find the transaction result (not the individual sample results)
+ SampleResult transactionResult = listener.getEvents().stream()
+ .map(e -> e.getResult())
+ .filter(TransactionController::isFromTransactionController)
+ .findFirst()
+ .orElse(null);
+ assertTrue(transactionResult != null,
+ "A transaction result should have been collected");
+ // The transaction elapsed time should be the sum of child sample times,
+ // not inflated by the timer delay between samples.
+ // Both child samples have very short simulated elapsed times (10ms each),
+ // so the total should be well under the timer delay (200ms).
+ assertTrue(transactionResult.getTime() < timerDelayMs,
+ "Transaction elapsed time (" + transactionResult.getTime() + " ms) should not include " +
+ "the timer delay (" + timerDelayMs + " ms)");
+ }
+
+ /**
+ * Test for GitHub issue #6496: transaction elapsed time should not include timer delay
+ * when the thread is stopped mid-transaction (e.g. during ramp-down), in parent mode
+ * (TransactionController with "Generate parent sample" checked).
+ *
+ * <p>The test uses a scheduler end time that expires before the timer delay completes,
+ * simulating a thread being stopped mid-transaction during ramp-down. The transaction
+ * elapsed time should equal the child sample elapsed time, not be inflated by the timer.
+ */
+ @Test
+ public void testIssue6496ParentMode() throws Exception {
+ JMeterContextService.getContext().setVariables(new JMeterVariables());
+
+ CollectSamplesListener listener = new CollectSamplesListener();
+
+ TransactionController transactionController = new TransactionController();
+ transactionController.setGenerateParentSample(true);
+ transactionController.setIncludeTimers(false);
+
+ long childElapsedMs = 10L;
+ long timerDelayMs = 5000L; // long timer that will be cut short by scheduler
+
+ FixedElapsedTimeSampler sampler = new FixedElapsedTimeSampler(childElapsedMs);
+ sampler.setName("Child Sampler");
+
+ FixedDelayTimer timer = new FixedDelayTimer(timerDelayMs);
+ timer.setName("Long Timer");
+ timer.setEnabled(true);
+
+ LoopController loop = new LoopController();
+ loop.setLoops(LoopController.INFINITE_LOOP_COUNT);
+ loop.setContinueForever(true);
+ loop.setEnabled(true);
+
+ ListedHashTree hashTree = new ListedHashTree();
+ hashTree.add(loop);
+ hashTree.add(loop, transactionController);
+ hashTree.add(transactionController, listener);
+ hashTree.add(transactionController, sampler);
+ hashTree.add(sampler, timer);
+
+ TestCompiler compiler = new TestCompiler(hashTree);
+ hashTree.traverse(compiler);
+
+ ThreadGroup threadGroup = new ThreadGroup();
+ threadGroup.setNumThreads(1);
+
+ ListenerNotifier notifier = new ListenerNotifier();
+
+ // Use scheduler to stop the thread after a short time (before the timer delay completes).
+ // The scheduler end time is set to expire before the timer delay, so adjustDelay() returns
+ // -1 and running is set to false immediately, simulating a thread stopped mid-transaction.
+ long maxDuration = 200L;
+ JMeterThread thread = new JMeterThread(hashTree, threadGroup, notifier);
+ thread.setScheduled(true);
+ thread.setEndTime(System.currentTimeMillis() + maxDuration);
+ thread.setThreadGroup(threadGroup);
+ thread.run();
+
+ assertFalse(listener.getEvents().isEmpty(),
+ "At least one transaction sample should have been collected");
+
+ // The last transaction event is the one interrupted during ramp-down.
+ // Its elapsed time should be close to the child sample elapsed time,
+ // not inflated by the long timer delay.
+ SampleResult lastTransaction = listener.getEvents().get(listener.getEvents().size() - 1).getResult();
Review Comment:
`CollectSamplesListener` keeps a reference to the `SampleResult`, and after the thread stops, `JMeterThread.run()` still calls `threadGroupLoopController.next()`, which calls `setTransactionDone()` on this same object. The values read here are therefore the ones computed after notification, not the ones a `ResultCollector` writes to the file. Please record `getTime()`, `getIdleTime()`, and `getResponseMessage()` inside `sampleOccurred` and assert on that snapshot.
##########
src/components/src/test/java/org/apache/jmeter/control/TestTransactionController.java:
##########
@@ -18,23 +18,224 @@
package org.apache.jmeter.control;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import org.apache.jmeter.assertions.ResponseAssertion;
import org.apache.jmeter.junit.JMeterTestCase;
import org.apache.jmeter.sampler.DebugSampler;
+import org.apache.jmeter.samplers.AbstractSampler;
+import org.apache.jmeter.samplers.Entry;
+import org.apache.jmeter.samplers.SampleResult;
import org.apache.jmeter.test.samplers.CollectSamplesListener;
+import org.apache.jmeter.testelement.AbstractTestElement;
import org.apache.jmeter.threads.JMeterContextService;
import org.apache.jmeter.threads.JMeterThread;
import org.apache.jmeter.threads.JMeterVariables;
import org.apache.jmeter.threads.ListenerNotifier;
import org.apache.jmeter.threads.TestCompiler;
import org.apache.jmeter.threads.ThreadGroup;
+import org.apache.jmeter.timers.Timer;
import org.apache.jorphan.collections.ListedHashTree;
import org.junit.jupiter.api.Test;
public class TestTransactionController extends JMeterTestCase {
+ /**
+ * A simple sampler that returns a successful result with a fixed elapsed time.
+ */
+ private static class FixedElapsedTimeSampler extends AbstractSampler {
+ private static final long serialVersionUID = 1L;
+ private final long elapsedTimeMs;
+
+ FixedElapsedTimeSampler(long elapsedTimeMs) {
+ this.elapsedTimeMs = elapsedTimeMs;
+ }
+
+ @Override
+ public SampleResult sample(Entry e) {
+ SampleResult result = new SampleResult();
+ result.setSampleLabel(getName());
+ result.sampleStart();
+ result.setSuccessful(true);
+ result.setResponseCodeOK();
+ // Simulate elapsed time by setting start/end times directly
+ long start = result.getStartTime();
+ result.setEndTime(start + elapsedTimeMs);
+ return result;
+ }
+ }
+
+ /**
+ * A timer that returns a fixed delay.
+ */
+ private static class FixedDelayTimer extends AbstractTestElement implements Timer {
+ private static final long serialVersionUID = 1L;
+ private final long delayMs;
+
+ FixedDelayTimer(long delayMs) {
+ this.delayMs = delayMs;
+ }
+
+ @Override
+ public long delay() {
+ return delayMs;
+ }
+ }
+
+ /**
+ * Test for GitHub issue #6496: transaction elapsed time should not include timer delay
+ * in non-parent mode (TransactionController with "Generate parent sample" unchecked).
+ *
+ * <p>The test sets up a transaction with two samplers and a timer between them.
+ * The transaction elapsed time should equal the sum of child sample elapsed times,
+ * not be inflated by the timer delay between samples.
+ */
+ @Test
+ public void testIssue6496NonParentMode() throws Exception {
+ JMeterContextService.getContext().setVariables(new JMeterVariables());
+
+ CollectSamplesListener listener = new CollectSamplesListener();
+
+ TransactionController transactionController = new TransactionController();
+ transactionController.setGenerateParentSample(false);
+ transactionController.setIncludeTimers(false);
+
+ // Use a simulated elapsed time much smaller than the timer delay
+ long childElapsedMs = 10L;
+ long timerDelayMs = 200L; // timer delay before second sampler
+
+ FixedElapsedTimeSampler firstSampler = new FixedElapsedTimeSampler(childElapsedMs);
+ firstSampler.setName("First Sampler");
+
+ FixedDelayTimer timer = new FixedDelayTimer(timerDelayMs);
+ timer.setName("Timer Before Second Sampler");
+ timer.setEnabled(true);
+
+ FixedElapsedTimeSampler secondSampler = new FixedElapsedTimeSampler(childElapsedMs);
+ secondSampler.setName("Second Sampler");
+
+ LoopController loop = new LoopController();
+ loop.setLoops(1);
+ loop.setContinueForever(false);
+ loop.setEnabled(true);
+
+ ListedHashTree hashTree = new ListedHashTree();
+ hashTree.add(loop);
+ hashTree.add(loop, transactionController);
+ // In non-parent mode, the TransactionController fires events using the SamplePackage
+ // of the last sampler that ran. The listener must be a child of that sampler.
+ hashTree.add(transactionController, firstSampler);
+ hashTree.add(transactionController, secondSampler);
+ hashTree.add(secondSampler, timer);
+ hashTree.add(secondSampler, listener);
+
+ TestCompiler compiler = new TestCompiler(hashTree);
+ hashTree.traverse(compiler);
+
+ ThreadGroup threadGroup = new ThreadGroup();
+ threadGroup.setNumThreads(1);
+
+ ListenerNotifier notifier = new ListenerNotifier();
+
+ JMeterThread thread = new JMeterThread(hashTree, threadGroup, notifier);
+ thread.setThreadGroup(threadGroup);
+ thread.run();
+
+ assertFalse(listener.getEvents().isEmpty(),
+ "At least one transaction sample should have been collected");
+
+ // Find the transaction result (not the individual sample results)
+ SampleResult transactionResult = listener.getEvents().stream()
+ .map(e -> e.getResult())
+ .filter(TransactionController::isFromTransactionController)
+ .findFirst()
+ .orElse(null);
+ assertTrue(transactionResult != null,
+ "A transaction result should have been collected");
+ // The transaction elapsed time should be the sum of child sample times,
+ // not inflated by the timer delay between samples.
+ // Both child samples have very short simulated elapsed times (10ms each),
+ // so the total should be well under the timer delay (200ms).
+ assertTrue(transactionResult.getTime() < timerDelayMs,
+ "Transaction elapsed time (" + transactionResult.getTime() + " ms) should not include " +
+ "the timer delay (" + timerDelayMs + " ms)");
+ }
+
+ /**
+ * Test for GitHub issue #6496: transaction elapsed time should not include timer delay
+ * when the thread is stopped mid-transaction (e.g. during ramp-down), in parent mode
+ * (TransactionController with "Generate parent sample" checked).
+ *
+ * <p>The test uses a scheduler end time that expires before the timer delay completes,
+ * simulating a thread being stopped mid-transaction during ramp-down. The transaction
+ * elapsed time should equal the child sample elapsed time, not be inflated by the timer.
+ */
+ @Test
+ public void testIssue6496ParentMode() throws Exception {
+ JMeterContextService.getContext().setVariables(new JMeterVariables());
+
+ CollectSamplesListener listener = new CollectSamplesListener();
+
+ TransactionController transactionController = new TransactionController();
+ transactionController.setGenerateParentSample(true);
+ transactionController.setIncludeTimers(false);
+
+ long childElapsedMs = 10L;
+ long timerDelayMs = 5000L; // long timer that will be cut short by scheduler
+
+ FixedElapsedTimeSampler sampler = new FixedElapsedTimeSampler(childElapsedMs);
+ sampler.setName("Child Sampler");
+
+ FixedDelayTimer timer = new FixedDelayTimer(timerDelayMs);
+ timer.setName("Long Timer");
+ timer.setEnabled(true);
+
+ LoopController loop = new LoopController();
+ loop.setLoops(LoopController.INFINITE_LOOP_COUNT);
+ loop.setContinueForever(true);
+ loop.setEnabled(true);
+
+ ListedHashTree hashTree = new ListedHashTree();
+ hashTree.add(loop);
+ hashTree.add(loop, transactionController);
+ hashTree.add(transactionController, listener);
+ hashTree.add(transactionController, sampler);
+ hashTree.add(sampler, timer);
Review Comment:
Same tree-building issue as in the other test: `hashTree.add(sampler, timer)` creates a top-level copy of `sampler`. Please use `tcTree.add(sampler).add(timer)`.
##########
src/components/src/test/java/org/apache/jmeter/control/TestTransactionController.java:
##########
@@ -18,23 +18,224 @@
package org.apache.jmeter.control;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import org.apache.jmeter.assertions.ResponseAssertion;
import org.apache.jmeter.junit.JMeterTestCase;
import org.apache.jmeter.sampler.DebugSampler;
+import org.apache.jmeter.samplers.AbstractSampler;
+import org.apache.jmeter.samplers.Entry;
+import org.apache.jmeter.samplers.SampleResult;
import org.apache.jmeter.test.samplers.CollectSamplesListener;
+import org.apache.jmeter.testelement.AbstractTestElement;
import org.apache.jmeter.threads.JMeterContextService;
import org.apache.jmeter.threads.JMeterThread;
import org.apache.jmeter.threads.JMeterVariables;
import org.apache.jmeter.threads.ListenerNotifier;
import org.apache.jmeter.threads.TestCompiler;
import org.apache.jmeter.threads.ThreadGroup;
+import org.apache.jmeter.timers.Timer;
import org.apache.jorphan.collections.ListedHashTree;
import org.junit.jupiter.api.Test;
public class TestTransactionController extends JMeterTestCase {
+ /**
+ * A simple sampler that returns a successful result with a fixed elapsed time.
+ */
+ private static class FixedElapsedTimeSampler extends AbstractSampler {
+ private static final long serialVersionUID = 1L;
+ private final long elapsedTimeMs;
+
+ FixedElapsedTimeSampler(long elapsedTimeMs) {
+ this.elapsedTimeMs = elapsedTimeMs;
+ }
+
+ @Override
+ public SampleResult sample(Entry e) {
+ SampleResult result = new SampleResult();
+ result.setSampleLabel(getName());
+ result.sampleStart();
+ result.setSuccessful(true);
+ result.setResponseCodeOK();
+ // Simulate elapsed time by setting start/end times directly
+ long start = result.getStartTime();
+ result.setEndTime(start + elapsedTimeMs);
+ return result;
+ }
+ }
+
+ /**
+ * A timer that returns a fixed delay.
+ */
+ private static class FixedDelayTimer extends AbstractTestElement implements Timer {
+ private static final long serialVersionUID = 1L;
+ private final long delayMs;
+
+ FixedDelayTimer(long delayMs) {
+ this.delayMs = delayMs;
+ }
+
+ @Override
+ public long delay() {
+ return delayMs;
+ }
+ }
+
+ /**
+ * Test for GitHub issue #6496: transaction elapsed time should not include timer delay
+ * in non-parent mode (TransactionController with "Generate parent sample" unchecked).
+ *
+ * <p>The test sets up a transaction with two samplers and a timer between them.
+ * The transaction elapsed time should equal the sum of child sample elapsed times,
+ * not be inflated by the timer delay between samples.
+ */
+ @Test
+ public void testIssue6496NonParentMode() throws Exception {
+ JMeterContextService.getContext().setVariables(new JMeterVariables());
+
+ CollectSamplesListener listener = new CollectSamplesListener();
+
+ TransactionController transactionController = new TransactionController();
+ transactionController.setGenerateParentSample(false);
+ transactionController.setIncludeTimers(false);
+
+ // Use a simulated elapsed time much smaller than the timer delay
+ long childElapsedMs = 10L;
+ long timerDelayMs = 200L; // timer delay before second sampler
+
+ FixedElapsedTimeSampler firstSampler = new FixedElapsedTimeSampler(childElapsedMs);
+ firstSampler.setName("First Sampler");
+
+ FixedDelayTimer timer = new FixedDelayTimer(timerDelayMs);
+ timer.setName("Timer Before Second Sampler");
+ timer.setEnabled(true);
+
+ FixedElapsedTimeSampler secondSampler = new FixedElapsedTimeSampler(childElapsedMs);
+ secondSampler.setName("Second Sampler");
+
+ LoopController loop = new LoopController();
+ loop.setLoops(1);
+ loop.setContinueForever(false);
+ loop.setEnabled(true);
+
+ ListedHashTree hashTree = new ListedHashTree();
+ hashTree.add(loop);
+ hashTree.add(loop, transactionController);
+ // In non-parent mode, the TransactionController fires events using the SamplePackage
+ // of the last sampler that ran. The listener must be a child of that sampler.
+ hashTree.add(transactionController, firstSampler);
+ hashTree.add(transactionController, secondSampler);
+ hashTree.add(secondSampler, timer);
+ hashTree.add(secondSampler, listener);
+
+ TestCompiler compiler = new TestCompiler(hashTree);
+ hashTree.traverse(compiler);
+
+ ThreadGroup threadGroup = new ThreadGroup();
+ threadGroup.setNumThreads(1);
+
+ ListenerNotifier notifier = new ListenerNotifier();
+
+ JMeterThread thread = new JMeterThread(hashTree, threadGroup, notifier);
+ thread.setThreadGroup(threadGroup);
+ thread.run();
+
+ assertFalse(listener.getEvents().isEmpty(),
+ "At least one transaction sample should have been collected");
+
+ // Find the transaction result (not the individual sample results)
+ SampleResult transactionResult = listener.getEvents().stream()
+ .map(e -> e.getResult())
+ .filter(TransactionController::isFromTransactionController)
+ .findFirst()
+ .orElse(null);
+ assertTrue(transactionResult != null,
+ "A transaction result should have been collected");
+ // The transaction elapsed time should be the sum of child sample times,
+ // not inflated by the timer delay between samples.
+ // Both child samples have very short simulated elapsed times (10ms each),
+ // so the total should be well under the timer delay (200ms).
+ assertTrue(transactionResult.getTime() < timerDelayMs,
+ "Transaction elapsed time (" + transactionResult.getTime() + " ms) should not include " +
+ "the timer delay (" + timerDelayMs + " ms)");
+ }
+
+ /**
+ * Test for GitHub issue #6496: transaction elapsed time should not include timer delay
+ * when the thread is stopped mid-transaction (e.g. during ramp-down), in parent mode
+ * (TransactionController with "Generate parent sample" checked).
+ *
+ * <p>The test uses a scheduler end time that expires before the timer delay completes,
+ * simulating a thread being stopped mid-transaction during ramp-down. The transaction
+ * elapsed time should equal the child sample elapsed time, not be inflated by the timer.
+ */
+ @Test
+ public void testIssue6496ParentMode() throws Exception {
+ JMeterContextService.getContext().setVariables(new JMeterVariables());
+
+ CollectSamplesListener listener = new CollectSamplesListener();
+
+ TransactionController transactionController = new TransactionController();
+ transactionController.setGenerateParentSample(true);
+ transactionController.setIncludeTimers(false);
+
+ long childElapsedMs = 10L;
+ long timerDelayMs = 5000L; // long timer that will be cut short by scheduler
+
+ FixedElapsedTimeSampler sampler = new FixedElapsedTimeSampler(childElapsedMs);
+ sampler.setName("Child Sampler");
+
+ FixedDelayTimer timer = new FixedDelayTimer(timerDelayMs);
+ timer.setName("Long Timer");
+ timer.setEnabled(true);
+
+ LoopController loop = new LoopController();
+ loop.setLoops(LoopController.INFINITE_LOOP_COUNT);
+ loop.setContinueForever(true);
+ loop.setEnabled(true);
+
+ ListedHashTree hashTree = new ListedHashTree();
+ hashTree.add(loop);
+ hashTree.add(loop, transactionController);
+ hashTree.add(transactionController, listener);
+ hashTree.add(transactionController, sampler);
+ hashTree.add(sampler, timer);
+
+ TestCompiler compiler = new TestCompiler(hashTree);
+ hashTree.traverse(compiler);
+
+ ThreadGroup threadGroup = new ThreadGroup();
+ threadGroup.setNumThreads(1);
+
+ ListenerNotifier notifier = new ListenerNotifier();
+
+ // Use scheduler to stop the thread after a short time (before the timer delay completes).
+ // The scheduler end time is set to expire before the timer delay, so adjustDelay() returns
+ // -1 and running is set to false immediately, simulating a thread stopped mid-transaction.
+ long maxDuration = 200L;
+ JMeterThread thread = new JMeterThread(hashTree, threadGroup, notifier);
+ thread.setScheduled(true);
+ thread.setEndTime(System.currentTimeMillis() + maxDuration);
+ thread.setThreadGroup(threadGroup);
+ thread.run();
+
+ assertFalse(listener.getEvents().isEmpty(),
+ "At least one transaction sample should have been collected");
+
+ // The last transaction event is the one interrupted during ramp-down.
+ // Its elapsed time should be close to the child sample elapsed time,
+ // not inflated by the long timer delay.
+ SampleResult lastTransaction = listener.getEvents().get(listener.getEvents().size() - 1).getResult();
+ assertTrue(TransactionController.isFromTransactionController(lastTransaction),
+ "Result should be from TransactionController");
+ assertTrue(lastTransaction.getTime() < timerDelayMs,
Review Comment:
Same as in the non-parent test: please assert the exact expected elapsed time and idle time with `assertEquals` instead of `< timerDelayMs`. The `assertTrue(isFromTransactionController(...))` above prints nothing useful on failure either; asserting the response message with `assertEquals` shows what was received.
##########
src/components/src/test/java/org/apache/jmeter/control/TestTransactionController.java:
##########
@@ -18,23 +18,224 @@
package org.apache.jmeter.control;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import org.apache.jmeter.assertions.ResponseAssertion;
import org.apache.jmeter.junit.JMeterTestCase;
import org.apache.jmeter.sampler.DebugSampler;
+import org.apache.jmeter.samplers.AbstractSampler;
+import org.apache.jmeter.samplers.Entry;
+import org.apache.jmeter.samplers.SampleResult;
import org.apache.jmeter.test.samplers.CollectSamplesListener;
+import org.apache.jmeter.testelement.AbstractTestElement;
import org.apache.jmeter.threads.JMeterContextService;
import org.apache.jmeter.threads.JMeterThread;
import org.apache.jmeter.threads.JMeterVariables;
import org.apache.jmeter.threads.ListenerNotifier;
import org.apache.jmeter.threads.TestCompiler;
import org.apache.jmeter.threads.ThreadGroup;
+import org.apache.jmeter.timers.Timer;
import org.apache.jorphan.collections.ListedHashTree;
import org.junit.jupiter.api.Test;
public class TestTransactionController extends JMeterTestCase {
+ /**
+ * A simple sampler that returns a successful result with a fixed elapsed time.
+ */
+ private static class FixedElapsedTimeSampler extends AbstractSampler {
+ private static final long serialVersionUID = 1L;
+ private final long elapsedTimeMs;
+
+ FixedElapsedTimeSampler(long elapsedTimeMs) {
+ this.elapsedTimeMs = elapsedTimeMs;
+ }
+
+ @Override
+ public SampleResult sample(Entry e) {
+ SampleResult result = new SampleResult();
+ result.setSampleLabel(getName());
+ result.sampleStart();
+ result.setSuccessful(true);
+ result.setResponseCodeOK();
+ // Simulate elapsed time by setting start/end times directly
+ long start = result.getStartTime();
+ result.setEndTime(start + elapsedTimeMs);
+ return result;
+ }
+ }
+
+ /**
+ * A timer that returns a fixed delay.
+ */
+ private static class FixedDelayTimer extends AbstractTestElement implements Timer {
+ private static final long serialVersionUID = 1L;
+ private final long delayMs;
+
+ FixedDelayTimer(long delayMs) {
+ this.delayMs = delayMs;
+ }
+
+ @Override
+ public long delay() {
+ return delayMs;
+ }
+ }
+
+ /**
+ * Test for GitHub issue #6496: transaction elapsed time should not include timer delay
+ * in non-parent mode (TransactionController with "Generate parent sample" unchecked).
+ *
+ * <p>The test sets up a transaction with two samplers and a timer between them.
+ * The transaction elapsed time should equal the sum of child sample elapsed times,
+ * not be inflated by the timer delay between samples.
+ */
+ @Test
+ public void testIssue6496NonParentMode() throws Exception {
+ JMeterContextService.getContext().setVariables(new JMeterVariables());
+
+ CollectSamplesListener listener = new CollectSamplesListener();
+
+ TransactionController transactionController = new TransactionController();
+ transactionController.setGenerateParentSample(false);
+ transactionController.setIncludeTimers(false);
+
+ // Use a simulated elapsed time much smaller than the timer delay
+ long childElapsedMs = 10L;
+ long timerDelayMs = 200L; // timer delay before second sampler
+
+ FixedElapsedTimeSampler firstSampler = new FixedElapsedTimeSampler(childElapsedMs);
+ firstSampler.setName("First Sampler");
+
+ FixedDelayTimer timer = new FixedDelayTimer(timerDelayMs);
+ timer.setName("Timer Before Second Sampler");
+ timer.setEnabled(true);
+
+ FixedElapsedTimeSampler secondSampler = new FixedElapsedTimeSampler(childElapsedMs);
+ secondSampler.setName("Second Sampler");
+
+ LoopController loop = new LoopController();
+ loop.setLoops(1);
+ loop.setContinueForever(false);
+ loop.setEnabled(true);
+
+ ListedHashTree hashTree = new ListedHashTree();
+ hashTree.add(loop);
+ hashTree.add(loop, transactionController);
+ // In non-parent mode, the TransactionController fires events using the SamplePackage
+ // of the last sampler that ran. The listener must be a child of that sampler.
+ hashTree.add(transactionController, firstSampler);
+ hashTree.add(transactionController, secondSampler);
+ hashTree.add(secondSampler, timer);
+ hashTree.add(secondSampler, listener);
+
+ TestCompiler compiler = new TestCompiler(hashTree);
+ hashTree.traverse(compiler);
+
+ ThreadGroup threadGroup = new ThreadGroup();
+ threadGroup.setNumThreads(1);
+
+ ListenerNotifier notifier = new ListenerNotifier();
+
+ JMeterThread thread = new JMeterThread(hashTree, threadGroup, notifier);
+ thread.setThreadGroup(threadGroup);
+ thread.run();
+
+ assertFalse(listener.getEvents().isEmpty(),
+ "At least one transaction sample should have been collected");
+
+ // Find the transaction result (not the individual sample results)
+ SampleResult transactionResult = listener.getEvents().stream()
+ .map(e -> e.getResult())
+ .filter(TransactionController::isFromTransactionController)
+ .findFirst()
+ .orElse(null);
+ assertTrue(transactionResult != null,
+ "A transaction result should have been collected");
+ // The transaction elapsed time should be the sum of child sample times,
+ // not inflated by the timer delay between samples.
+ // Both child samples have very short simulated elapsed times (10ms each),
+ // so the total should be well under the timer delay (200ms).
+ assertTrue(transactionResult.getTime() < timerDelayMs,
Review Comment:
`time < timerDelayMs` would still pass if the transaction included half the timer. The children have fixed elapsed times, so the expected value is known: please use `assertEquals(2 * childElapsedMs, transactionResult.getTime(), ...)` (and assert the idle time as well, since the fix moves the delay there). The value must be captured inside the listener's `sampleOccurred`, because the `SampleResult` is mutated after notification (see the review summary).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixed issue where transaction sample elapsed time was incorrectly inflated when a thread was stopped [jmeter]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on PR #6770:
URL: https://github.com/apache/jmeter/pull/6770#issuecomment-6349880108
@vlsi, I believe I've implemented all of your recommended changes. Are there any additional changes you would like me to make?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixed issue where transaction sample elapsed time was incorrectly inflated when a thread was stopped [jmeter]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on code in PR #6770:
URL: https://github.com/apache/jmeter/pull/6770#discussion_r4106388987
##########
.github/workflows/gradle-wrapper-validation.yml:
##########
@@ -7,4 +7,4 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v6
- - uses: gradle/actions/wrapper-validation@0723195856401067f7a2779048b490ace7a47d7c # v5.0.2
+ - uses: gradle/actions/wrapper-validation@3f131e8634966bd73d06cc69884922b02e6faf92 # v6.2.0
Review Comment:
Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixed issue where transaction sample elapsed time was incorrectly inflated when a thread was stopped [jmeter]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on code in PR #6770:
URL: https://github.com/apache/jmeter/pull/6770#discussion_r4106385290
##########
src/core/src/main/java/org/apache/jmeter/control/TransactionController.java:
##########
@@ -252,6 +252,14 @@ public static boolean isFromTransactionController(SampleResult res) {
public void triggerEndOfLoop() {
if(!isGenerateParentSample()) {
if (res != null) {
+ // See BUG 55816 / GitHub issue #6496
Review Comment:
Fixed
##########
src/core/src/main/java/org/apache/jmeter/control/TransactionSampler.java:
##########
@@ -119,7 +119,7 @@ public void addSubSamplerResult(SampleResult res) {
totalConnectTime += res.getConnectTime();
}
- protected void setTransactionDone() {
+ public void setTransactionDone() {
Review Comment:
Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixed issue where transaction sample elapsed time was incorrectly inflated when a thread was stopped [jmeter]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on code in PR #6770:
URL: https://github.com/apache/jmeter/pull/6770#discussion_r4106395551
##########
src/components/src/test/java/org/apache/jmeter/control/TestTransactionController.java:
##########
@@ -18,23 +18,224 @@
package org.apache.jmeter.control;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import org.apache.jmeter.assertions.ResponseAssertion;
import org.apache.jmeter.junit.JMeterTestCase;
import org.apache.jmeter.sampler.DebugSampler;
+import org.apache.jmeter.samplers.AbstractSampler;
+import org.apache.jmeter.samplers.Entry;
+import org.apache.jmeter.samplers.SampleResult;
import org.apache.jmeter.test.samplers.CollectSamplesListener;
+import org.apache.jmeter.testelement.AbstractTestElement;
import org.apache.jmeter.threads.JMeterContextService;
import org.apache.jmeter.threads.JMeterThread;
import org.apache.jmeter.threads.JMeterVariables;
import org.apache.jmeter.threads.ListenerNotifier;
import org.apache.jmeter.threads.TestCompiler;
import org.apache.jmeter.threads.ThreadGroup;
+import org.apache.jmeter.timers.Timer;
import org.apache.jorphan.collections.ListedHashTree;
import org.junit.jupiter.api.Test;
public class TestTransactionController extends JMeterTestCase {
+ /**
+ * A simple sampler that returns a successful result with a fixed elapsed time.
+ */
+ private static class FixedElapsedTimeSampler extends AbstractSampler {
+ private static final long serialVersionUID = 1L;
+ private final long elapsedTimeMs;
+
+ FixedElapsedTimeSampler(long elapsedTimeMs) {
+ this.elapsedTimeMs = elapsedTimeMs;
+ }
+
+ @Override
+ public SampleResult sample(Entry e) {
+ SampleResult result = new SampleResult();
+ result.setSampleLabel(getName());
+ result.sampleStart();
+ result.setSuccessful(true);
+ result.setResponseCodeOK();
+ // Simulate elapsed time by setting start/end times directly
+ long start = result.getStartTime();
+ result.setEndTime(start + elapsedTimeMs);
+ return result;
+ }
+ }
+
+ /**
+ * A timer that returns a fixed delay.
+ */
+ private static class FixedDelayTimer extends AbstractTestElement implements Timer {
+ private static final long serialVersionUID = 1L;
+ private final long delayMs;
+
+ FixedDelayTimer(long delayMs) {
+ this.delayMs = delayMs;
+ }
+
+ @Override
+ public long delay() {
+ return delayMs;
+ }
+ }
+
+ /**
+ * Test for GitHub issue #6496: transaction elapsed time should not include timer delay
+ * in non-parent mode (TransactionController with "Generate parent sample" unchecked).
+ *
+ * <p>The test sets up a transaction with two samplers and a timer between them.
+ * The transaction elapsed time should equal the sum of child sample elapsed times,
+ * not be inflated by the timer delay between samples.
+ */
+ @Test
+ public void testIssue6496NonParentMode() throws Exception {
+ JMeterContextService.getContext().setVariables(new JMeterVariables());
+
+ CollectSamplesListener listener = new CollectSamplesListener();
+
+ TransactionController transactionController = new TransactionController();
+ transactionController.setGenerateParentSample(false);
+ transactionController.setIncludeTimers(false);
+
+ // Use a simulated elapsed time much smaller than the timer delay
+ long childElapsedMs = 10L;
+ long timerDelayMs = 200L; // timer delay before second sampler
+
+ FixedElapsedTimeSampler firstSampler = new FixedElapsedTimeSampler(childElapsedMs);
+ firstSampler.setName("First Sampler");
+
+ FixedDelayTimer timer = new FixedDelayTimer(timerDelayMs);
+ timer.setName("Timer Before Second Sampler");
+ timer.setEnabled(true);
+
+ FixedElapsedTimeSampler secondSampler = new FixedElapsedTimeSampler(childElapsedMs);
+ secondSampler.setName("Second Sampler");
+
+ LoopController loop = new LoopController();
+ loop.setLoops(1);
+ loop.setContinueForever(false);
+ loop.setEnabled(true);
+
+ ListedHashTree hashTree = new ListedHashTree();
+ hashTree.add(loop);
+ hashTree.add(loop, transactionController);
+ // In non-parent mode, the TransactionController fires events using the SamplePackage
+ // of the last sampler that ran. The listener must be a child of that sampler.
+ hashTree.add(transactionController, firstSampler);
+ hashTree.add(transactionController, secondSampler);
+ hashTree.add(secondSampler, timer);
+ hashTree.add(secondSampler, listener);
+
+ TestCompiler compiler = new TestCompiler(hashTree);
+ hashTree.traverse(compiler);
+
+ ThreadGroup threadGroup = new ThreadGroup();
+ threadGroup.setNumThreads(1);
+
+ ListenerNotifier notifier = new ListenerNotifier();
+
+ JMeterThread thread = new JMeterThread(hashTree, threadGroup, notifier);
+ thread.setThreadGroup(threadGroup);
+ thread.run();
+
+ assertFalse(listener.getEvents().isEmpty(),
+ "At least one transaction sample should have been collected");
+
+ // Find the transaction result (not the individual sample results)
+ SampleResult transactionResult = listener.getEvents().stream()
+ .map(e -> e.getResult())
+ .filter(TransactionController::isFromTransactionController)
+ .findFirst()
+ .orElse(null);
+ assertTrue(transactionResult != null,
+ "A transaction result should have been collected");
+ // The transaction elapsed time should be the sum of child sample times,
+ // not inflated by the timer delay between samples.
+ // Both child samples have very short simulated elapsed times (10ms each),
+ // so the total should be well under the timer delay (200ms).
+ assertTrue(transactionResult.getTime() < timerDelayMs,
+ "Transaction elapsed time (" + transactionResult.getTime() + " ms) should not include " +
+ "the timer delay (" + timerDelayMs + " ms)");
+ }
+
+ /**
+ * Test for GitHub issue #6496: transaction elapsed time should not include timer delay
+ * when the thread is stopped mid-transaction (e.g. during ramp-down), in parent mode
+ * (TransactionController with "Generate parent sample" checked).
+ *
+ * <p>The test uses a scheduler end time that expires before the timer delay completes,
+ * simulating a thread being stopped mid-transaction during ramp-down. The transaction
+ * elapsed time should equal the child sample elapsed time, not be inflated by the timer.
+ */
+ @Test
+ public void testIssue6496ParentMode() throws Exception {
+ JMeterContextService.getContext().setVariables(new JMeterVariables());
+
+ CollectSamplesListener listener = new CollectSamplesListener();
+
+ TransactionController transactionController = new TransactionController();
+ transactionController.setGenerateParentSample(true);
+ transactionController.setIncludeTimers(false);
+
+ long childElapsedMs = 10L;
+ long timerDelayMs = 5000L; // long timer that will be cut short by scheduler
+
+ FixedElapsedTimeSampler sampler = new FixedElapsedTimeSampler(childElapsedMs);
+ sampler.setName("Child Sampler");
+
+ FixedDelayTimer timer = new FixedDelayTimer(timerDelayMs);
+ timer.setName("Long Timer");
+ timer.setEnabled(true);
+
+ LoopController loop = new LoopController();
+ loop.setLoops(LoopController.INFINITE_LOOP_COUNT);
+ loop.setContinueForever(true);
+ loop.setEnabled(true);
+
+ ListedHashTree hashTree = new ListedHashTree();
+ hashTree.add(loop);
+ hashTree.add(loop, transactionController);
+ hashTree.add(transactionController, listener);
+ hashTree.add(transactionController, sampler);
+ hashTree.add(sampler, timer);
+
+ TestCompiler compiler = new TestCompiler(hashTree);
+ hashTree.traverse(compiler);
+
+ ThreadGroup threadGroup = new ThreadGroup();
+ threadGroup.setNumThreads(1);
+
+ ListenerNotifier notifier = new ListenerNotifier();
+
+ // Use scheduler to stop the thread after a short time (before the timer delay completes).
+ // The scheduler end time is set to expire before the timer delay, so adjustDelay() returns
+ // -1 and running is set to false immediately, simulating a thread stopped mid-transaction.
+ long maxDuration = 200L;
+ JMeterThread thread = new JMeterThread(hashTree, threadGroup, notifier);
+ thread.setScheduled(true);
+ thread.setEndTime(System.currentTimeMillis() + maxDuration);
+ thread.setThreadGroup(threadGroup);
+ thread.run();
+
+ assertFalse(listener.getEvents().isEmpty(),
+ "At least one transaction sample should have been collected");
+
+ // The last transaction event is the one interrupted during ramp-down.
+ // Its elapsed time should be close to the child sample elapsed time,
+ // not inflated by the long timer delay.
+ SampleResult lastTransaction = listener.getEvents().get(listener.getEvents().size() - 1).getResult();
Review Comment:
Fixed
##########
src/components/src/test/java/org/apache/jmeter/control/TestTransactionController.java:
##########
@@ -18,23 +18,224 @@
package org.apache.jmeter.control;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import org.apache.jmeter.assertions.ResponseAssertion;
import org.apache.jmeter.junit.JMeterTestCase;
import org.apache.jmeter.sampler.DebugSampler;
+import org.apache.jmeter.samplers.AbstractSampler;
+import org.apache.jmeter.samplers.Entry;
+import org.apache.jmeter.samplers.SampleResult;
import org.apache.jmeter.test.samplers.CollectSamplesListener;
+import org.apache.jmeter.testelement.AbstractTestElement;
import org.apache.jmeter.threads.JMeterContextService;
import org.apache.jmeter.threads.JMeterThread;
import org.apache.jmeter.threads.JMeterVariables;
import org.apache.jmeter.threads.ListenerNotifier;
import org.apache.jmeter.threads.TestCompiler;
import org.apache.jmeter.threads.ThreadGroup;
+import org.apache.jmeter.timers.Timer;
import org.apache.jorphan.collections.ListedHashTree;
import org.junit.jupiter.api.Test;
public class TestTransactionController extends JMeterTestCase {
+ /**
+ * A simple sampler that returns a successful result with a fixed elapsed time.
+ */
+ private static class FixedElapsedTimeSampler extends AbstractSampler {
+ private static final long serialVersionUID = 1L;
+ private final long elapsedTimeMs;
+
+ FixedElapsedTimeSampler(long elapsedTimeMs) {
+ this.elapsedTimeMs = elapsedTimeMs;
+ }
+
+ @Override
+ public SampleResult sample(Entry e) {
+ SampleResult result = new SampleResult();
+ result.setSampleLabel(getName());
+ result.sampleStart();
+ result.setSuccessful(true);
+ result.setResponseCodeOK();
+ // Simulate elapsed time by setting start/end times directly
+ long start = result.getStartTime();
+ result.setEndTime(start + elapsedTimeMs);
+ return result;
+ }
+ }
+
+ /**
+ * A timer that returns a fixed delay.
+ */
+ private static class FixedDelayTimer extends AbstractTestElement implements Timer {
+ private static final long serialVersionUID = 1L;
+ private final long delayMs;
+
+ FixedDelayTimer(long delayMs) {
+ this.delayMs = delayMs;
+ }
+
+ @Override
+ public long delay() {
+ return delayMs;
+ }
+ }
+
+ /**
+ * Test for GitHub issue #6496: transaction elapsed time should not include timer delay
+ * in non-parent mode (TransactionController with "Generate parent sample" unchecked).
+ *
+ * <p>The test sets up a transaction with two samplers and a timer between them.
+ * The transaction elapsed time should equal the sum of child sample elapsed times,
+ * not be inflated by the timer delay between samples.
+ */
+ @Test
+ public void testIssue6496NonParentMode() throws Exception {
+ JMeterContextService.getContext().setVariables(new JMeterVariables());
+
+ CollectSamplesListener listener = new CollectSamplesListener();
+
+ TransactionController transactionController = new TransactionController();
+ transactionController.setGenerateParentSample(false);
+ transactionController.setIncludeTimers(false);
+
+ // Use a simulated elapsed time much smaller than the timer delay
+ long childElapsedMs = 10L;
+ long timerDelayMs = 200L; // timer delay before second sampler
+
+ FixedElapsedTimeSampler firstSampler = new FixedElapsedTimeSampler(childElapsedMs);
+ firstSampler.setName("First Sampler");
+
+ FixedDelayTimer timer = new FixedDelayTimer(timerDelayMs);
+ timer.setName("Timer Before Second Sampler");
+ timer.setEnabled(true);
+
+ FixedElapsedTimeSampler secondSampler = new FixedElapsedTimeSampler(childElapsedMs);
+ secondSampler.setName("Second Sampler");
+
+ LoopController loop = new LoopController();
+ loop.setLoops(1);
+ loop.setContinueForever(false);
+ loop.setEnabled(true);
+
+ ListedHashTree hashTree = new ListedHashTree();
+ hashTree.add(loop);
+ hashTree.add(loop, transactionController);
+ // In non-parent mode, the TransactionController fires events using the SamplePackage
+ // of the last sampler that ran. The listener must be a child of that sampler.
+ hashTree.add(transactionController, firstSampler);
+ hashTree.add(transactionController, secondSampler);
+ hashTree.add(secondSampler, timer);
+ hashTree.add(secondSampler, listener);
+
+ TestCompiler compiler = new TestCompiler(hashTree);
+ hashTree.traverse(compiler);
+
+ ThreadGroup threadGroup = new ThreadGroup();
+ threadGroup.setNumThreads(1);
+
+ ListenerNotifier notifier = new ListenerNotifier();
+
+ JMeterThread thread = new JMeterThread(hashTree, threadGroup, notifier);
+ thread.setThreadGroup(threadGroup);
+ thread.run();
+
+ assertFalse(listener.getEvents().isEmpty(),
+ "At least one transaction sample should have been collected");
+
+ // Find the transaction result (not the individual sample results)
+ SampleResult transactionResult = listener.getEvents().stream()
+ .map(e -> e.getResult())
+ .filter(TransactionController::isFromTransactionController)
+ .findFirst()
+ .orElse(null);
+ assertTrue(transactionResult != null,
+ "A transaction result should have been collected");
+ // The transaction elapsed time should be the sum of child sample times,
+ // not inflated by the timer delay between samples.
+ // Both child samples have very short simulated elapsed times (10ms each),
+ // so the total should be well under the timer delay (200ms).
+ assertTrue(transactionResult.getTime() < timerDelayMs,
+ "Transaction elapsed time (" + transactionResult.getTime() + " ms) should not include " +
+ "the timer delay (" + timerDelayMs + " ms)");
+ }
+
+ /**
+ * Test for GitHub issue #6496: transaction elapsed time should not include timer delay
+ * when the thread is stopped mid-transaction (e.g. during ramp-down), in parent mode
+ * (TransactionController with "Generate parent sample" checked).
+ *
+ * <p>The test uses a scheduler end time that expires before the timer delay completes,
+ * simulating a thread being stopped mid-transaction during ramp-down. The transaction
+ * elapsed time should equal the child sample elapsed time, not be inflated by the timer.
+ */
+ @Test
+ public void testIssue6496ParentMode() throws Exception {
+ JMeterContextService.getContext().setVariables(new JMeterVariables());
+
+ CollectSamplesListener listener = new CollectSamplesListener();
+
+ TransactionController transactionController = new TransactionController();
+ transactionController.setGenerateParentSample(true);
+ transactionController.setIncludeTimers(false);
+
+ long childElapsedMs = 10L;
+ long timerDelayMs = 5000L; // long timer that will be cut short by scheduler
+
+ FixedElapsedTimeSampler sampler = new FixedElapsedTimeSampler(childElapsedMs);
+ sampler.setName("Child Sampler");
+
+ FixedDelayTimer timer = new FixedDelayTimer(timerDelayMs);
+ timer.setName("Long Timer");
+ timer.setEnabled(true);
+
+ LoopController loop = new LoopController();
+ loop.setLoops(LoopController.INFINITE_LOOP_COUNT);
+ loop.setContinueForever(true);
+ loop.setEnabled(true);
+
+ ListedHashTree hashTree = new ListedHashTree();
+ hashTree.add(loop);
+ hashTree.add(loop, transactionController);
+ hashTree.add(transactionController, listener);
+ hashTree.add(transactionController, sampler);
+ hashTree.add(sampler, timer);
+
+ TestCompiler compiler = new TestCompiler(hashTree);
+ hashTree.traverse(compiler);
+
+ ThreadGroup threadGroup = new ThreadGroup();
+ threadGroup.setNumThreads(1);
+
+ ListenerNotifier notifier = new ListenerNotifier();
+
+ // Use scheduler to stop the thread after a short time (before the timer delay completes).
+ // The scheduler end time is set to expire before the timer delay, so adjustDelay() returns
+ // -1 and running is set to false immediately, simulating a thread stopped mid-transaction.
+ long maxDuration = 200L;
+ JMeterThread thread = new JMeterThread(hashTree, threadGroup, notifier);
+ thread.setScheduled(true);
+ thread.setEndTime(System.currentTimeMillis() + maxDuration);
+ thread.setThreadGroup(threadGroup);
+ thread.run();
+
+ assertFalse(listener.getEvents().isEmpty(),
+ "At least one transaction sample should have been collected");
+
+ // The last transaction event is the one interrupted during ramp-down.
+ // Its elapsed time should be close to the child sample elapsed time,
+ // not inflated by the long timer delay.
+ SampleResult lastTransaction = listener.getEvents().get(listener.getEvents().size() - 1).getResult();
+ assertTrue(TransactionController.isFromTransactionController(lastTransaction),
+ "Result should be from TransactionController");
+ assertTrue(lastTransaction.getTime() < timerDelayMs,
Review Comment:
Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixed issue where transaction sample elapsed time was incorrectly inflated when a thread was stopped [jmeter]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on code in PR #6770:
URL: https://github.com/apache/jmeter/pull/6770#discussion_r4106392820
##########
src/components/src/test/java/org/apache/jmeter/control/TestTransactionController.java:
##########
@@ -18,23 +18,224 @@
package org.apache.jmeter.control;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import org.apache.jmeter.assertions.ResponseAssertion;
import org.apache.jmeter.junit.JMeterTestCase;
import org.apache.jmeter.sampler.DebugSampler;
+import org.apache.jmeter.samplers.AbstractSampler;
+import org.apache.jmeter.samplers.Entry;
+import org.apache.jmeter.samplers.SampleResult;
import org.apache.jmeter.test.samplers.CollectSamplesListener;
+import org.apache.jmeter.testelement.AbstractTestElement;
import org.apache.jmeter.threads.JMeterContextService;
import org.apache.jmeter.threads.JMeterThread;
import org.apache.jmeter.threads.JMeterVariables;
import org.apache.jmeter.threads.ListenerNotifier;
import org.apache.jmeter.threads.TestCompiler;
import org.apache.jmeter.threads.ThreadGroup;
+import org.apache.jmeter.timers.Timer;
import org.apache.jorphan.collections.ListedHashTree;
import org.junit.jupiter.api.Test;
public class TestTransactionController extends JMeterTestCase {
+ /**
+ * A simple sampler that returns a successful result with a fixed elapsed time.
+ */
+ private static class FixedElapsedTimeSampler extends AbstractSampler {
+ private static final long serialVersionUID = 1L;
+ private final long elapsedTimeMs;
+
+ FixedElapsedTimeSampler(long elapsedTimeMs) {
+ this.elapsedTimeMs = elapsedTimeMs;
+ }
+
+ @Override
+ public SampleResult sample(Entry e) {
+ SampleResult result = new SampleResult();
+ result.setSampleLabel(getName());
+ result.sampleStart();
+ result.setSuccessful(true);
+ result.setResponseCodeOK();
+ // Simulate elapsed time by setting start/end times directly
+ long start = result.getStartTime();
+ result.setEndTime(start + elapsedTimeMs);
+ return result;
+ }
+ }
+
+ /**
+ * A timer that returns a fixed delay.
+ */
+ private static class FixedDelayTimer extends AbstractTestElement implements Timer {
+ private static final long serialVersionUID = 1L;
+ private final long delayMs;
+
+ FixedDelayTimer(long delayMs) {
+ this.delayMs = delayMs;
+ }
+
+ @Override
+ public long delay() {
+ return delayMs;
+ }
+ }
+
+ /**
+ * Test for GitHub issue #6496: transaction elapsed time should not include timer delay
+ * in non-parent mode (TransactionController with "Generate parent sample" unchecked).
+ *
+ * <p>The test sets up a transaction with two samplers and a timer between them.
+ * The transaction elapsed time should equal the sum of child sample elapsed times,
+ * not be inflated by the timer delay between samples.
+ */
+ @Test
+ public void testIssue6496NonParentMode() throws Exception {
+ JMeterContextService.getContext().setVariables(new JMeterVariables());
+
+ CollectSamplesListener listener = new CollectSamplesListener();
+
+ TransactionController transactionController = new TransactionController();
+ transactionController.setGenerateParentSample(false);
+ transactionController.setIncludeTimers(false);
+
+ // Use a simulated elapsed time much smaller than the timer delay
+ long childElapsedMs = 10L;
+ long timerDelayMs = 200L; // timer delay before second sampler
+
+ FixedElapsedTimeSampler firstSampler = new FixedElapsedTimeSampler(childElapsedMs);
+ firstSampler.setName("First Sampler");
+
+ FixedDelayTimer timer = new FixedDelayTimer(timerDelayMs);
+ timer.setName("Timer Before Second Sampler");
+ timer.setEnabled(true);
+
+ FixedElapsedTimeSampler secondSampler = new FixedElapsedTimeSampler(childElapsedMs);
+ secondSampler.setName("Second Sampler");
+
+ LoopController loop = new LoopController();
+ loop.setLoops(1);
+ loop.setContinueForever(false);
+ loop.setEnabled(true);
+
+ ListedHashTree hashTree = new ListedHashTree();
+ hashTree.add(loop);
+ hashTree.add(loop, transactionController);
+ // In non-parent mode, the TransactionController fires events using the SamplePackage
+ // of the last sampler that ran. The listener must be a child of that sampler.
+ hashTree.add(transactionController, firstSampler);
+ hashTree.add(transactionController, secondSampler);
+ hashTree.add(secondSampler, timer);
+ hashTree.add(secondSampler, listener);
+
+ TestCompiler compiler = new TestCompiler(hashTree);
+ hashTree.traverse(compiler);
+
+ ThreadGroup threadGroup = new ThreadGroup();
+ threadGroup.setNumThreads(1);
+
+ ListenerNotifier notifier = new ListenerNotifier();
+
+ JMeterThread thread = new JMeterThread(hashTree, threadGroup, notifier);
+ thread.setThreadGroup(threadGroup);
+ thread.run();
+
+ assertFalse(listener.getEvents().isEmpty(),
+ "At least one transaction sample should have been collected");
+
+ // Find the transaction result (not the individual sample results)
+ SampleResult transactionResult = listener.getEvents().stream()
+ .map(e -> e.getResult())
+ .filter(TransactionController::isFromTransactionController)
+ .findFirst()
+ .orElse(null);
+ assertTrue(transactionResult != null,
+ "A transaction result should have been collected");
+ // The transaction elapsed time should be the sum of child sample times,
+ // not inflated by the timer delay between samples.
+ // Both child samples have very short simulated elapsed times (10ms each),
+ // so the total should be well under the timer delay (200ms).
+ assertTrue(transactionResult.getTime() < timerDelayMs,
+ "Transaction elapsed time (" + transactionResult.getTime() + " ms) should not include " +
+ "the timer delay (" + timerDelayMs + " ms)");
+ }
+
+ /**
+ * Test for GitHub issue #6496: transaction elapsed time should not include timer delay
+ * when the thread is stopped mid-transaction (e.g. during ramp-down), in parent mode
+ * (TransactionController with "Generate parent sample" checked).
+ *
+ * <p>The test uses a scheduler end time that expires before the timer delay completes,
+ * simulating a thread being stopped mid-transaction during ramp-down. The transaction
+ * elapsed time should equal the child sample elapsed time, not be inflated by the timer.
+ */
+ @Test
+ public void testIssue6496ParentMode() throws Exception {
Review Comment:
Fixed
##########
src/components/src/test/java/org/apache/jmeter/control/TestTransactionController.java:
##########
@@ -18,23 +18,224 @@
package org.apache.jmeter.control;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import org.apache.jmeter.assertions.ResponseAssertion;
import org.apache.jmeter.junit.JMeterTestCase;
import org.apache.jmeter.sampler.DebugSampler;
+import org.apache.jmeter.samplers.AbstractSampler;
+import org.apache.jmeter.samplers.Entry;
+import org.apache.jmeter.samplers.SampleResult;
import org.apache.jmeter.test.samplers.CollectSamplesListener;
+import org.apache.jmeter.testelement.AbstractTestElement;
import org.apache.jmeter.threads.JMeterContextService;
import org.apache.jmeter.threads.JMeterThread;
import org.apache.jmeter.threads.JMeterVariables;
import org.apache.jmeter.threads.ListenerNotifier;
import org.apache.jmeter.threads.TestCompiler;
import org.apache.jmeter.threads.ThreadGroup;
+import org.apache.jmeter.timers.Timer;
import org.apache.jorphan.collections.ListedHashTree;
import org.junit.jupiter.api.Test;
public class TestTransactionController extends JMeterTestCase {
+ /**
+ * A simple sampler that returns a successful result with a fixed elapsed time.
+ */
+ private static class FixedElapsedTimeSampler extends AbstractSampler {
+ private static final long serialVersionUID = 1L;
+ private final long elapsedTimeMs;
+
+ FixedElapsedTimeSampler(long elapsedTimeMs) {
+ this.elapsedTimeMs = elapsedTimeMs;
+ }
+
+ @Override
+ public SampleResult sample(Entry e) {
+ SampleResult result = new SampleResult();
+ result.setSampleLabel(getName());
+ result.sampleStart();
+ result.setSuccessful(true);
+ result.setResponseCodeOK();
+ // Simulate elapsed time by setting start/end times directly
+ long start = result.getStartTime();
+ result.setEndTime(start + elapsedTimeMs);
+ return result;
+ }
+ }
+
+ /**
+ * A timer that returns a fixed delay.
+ */
+ private static class FixedDelayTimer extends AbstractTestElement implements Timer {
+ private static final long serialVersionUID = 1L;
+ private final long delayMs;
+
+ FixedDelayTimer(long delayMs) {
+ this.delayMs = delayMs;
+ }
+
+ @Override
+ public long delay() {
+ return delayMs;
+ }
+ }
+
+ /**
+ * Test for GitHub issue #6496: transaction elapsed time should not include timer delay
+ * in non-parent mode (TransactionController with "Generate parent sample" unchecked).
+ *
+ * <p>The test sets up a transaction with two samplers and a timer between them.
+ * The transaction elapsed time should equal the sum of child sample elapsed times,
+ * not be inflated by the timer delay between samples.
+ */
+ @Test
+ public void testIssue6496NonParentMode() throws Exception {
+ JMeterContextService.getContext().setVariables(new JMeterVariables());
+
+ CollectSamplesListener listener = new CollectSamplesListener();
+
+ TransactionController transactionController = new TransactionController();
+ transactionController.setGenerateParentSample(false);
+ transactionController.setIncludeTimers(false);
+
+ // Use a simulated elapsed time much smaller than the timer delay
+ long childElapsedMs = 10L;
+ long timerDelayMs = 200L; // timer delay before second sampler
+
+ FixedElapsedTimeSampler firstSampler = new FixedElapsedTimeSampler(childElapsedMs);
+ firstSampler.setName("First Sampler");
+
+ FixedDelayTimer timer = new FixedDelayTimer(timerDelayMs);
+ timer.setName("Timer Before Second Sampler");
+ timer.setEnabled(true);
+
+ FixedElapsedTimeSampler secondSampler = new FixedElapsedTimeSampler(childElapsedMs);
+ secondSampler.setName("Second Sampler");
+
+ LoopController loop = new LoopController();
+ loop.setLoops(1);
+ loop.setContinueForever(false);
+ loop.setEnabled(true);
+
+ ListedHashTree hashTree = new ListedHashTree();
+ hashTree.add(loop);
+ hashTree.add(loop, transactionController);
+ // In non-parent mode, the TransactionController fires events using the SamplePackage
+ // of the last sampler that ran. The listener must be a child of that sampler.
+ hashTree.add(transactionController, firstSampler);
+ hashTree.add(transactionController, secondSampler);
+ hashTree.add(secondSampler, timer);
+ hashTree.add(secondSampler, listener);
+
+ TestCompiler compiler = new TestCompiler(hashTree);
+ hashTree.traverse(compiler);
+
+ ThreadGroup threadGroup = new ThreadGroup();
+ threadGroup.setNumThreads(1);
+
+ ListenerNotifier notifier = new ListenerNotifier();
+
+ JMeterThread thread = new JMeterThread(hashTree, threadGroup, notifier);
+ thread.setThreadGroup(threadGroup);
+ thread.run();
+
+ assertFalse(listener.getEvents().isEmpty(),
+ "At least one transaction sample should have been collected");
+
+ // Find the transaction result (not the individual sample results)
+ SampleResult transactionResult = listener.getEvents().stream()
+ .map(e -> e.getResult())
+ .filter(TransactionController::isFromTransactionController)
+ .findFirst()
+ .orElse(null);
+ assertTrue(transactionResult != null,
+ "A transaction result should have been collected");
+ // The transaction elapsed time should be the sum of child sample times,
+ // not inflated by the timer delay between samples.
+ // Both child samples have very short simulated elapsed times (10ms each),
+ // so the total should be well under the timer delay (200ms).
+ assertTrue(transactionResult.getTime() < timerDelayMs,
+ "Transaction elapsed time (" + transactionResult.getTime() + " ms) should not include " +
+ "the timer delay (" + timerDelayMs + " ms)");
+ }
+
+ /**
+ * Test for GitHub issue #6496: transaction elapsed time should not include timer delay
+ * when the thread is stopped mid-transaction (e.g. during ramp-down), in parent mode
+ * (TransactionController with "Generate parent sample" checked).
+ *
+ * <p>The test uses a scheduler end time that expires before the timer delay completes,
+ * simulating a thread being stopped mid-transaction during ramp-down. The transaction
+ * elapsed time should equal the child sample elapsed time, not be inflated by the timer.
+ */
+ @Test
+ public void testIssue6496ParentMode() throws Exception {
+ JMeterContextService.getContext().setVariables(new JMeterVariables());
+
+ CollectSamplesListener listener = new CollectSamplesListener();
+
+ TransactionController transactionController = new TransactionController();
+ transactionController.setGenerateParentSample(true);
+ transactionController.setIncludeTimers(false);
+
+ long childElapsedMs = 10L;
+ long timerDelayMs = 5000L; // long timer that will be cut short by scheduler
+
+ FixedElapsedTimeSampler sampler = new FixedElapsedTimeSampler(childElapsedMs);
+ sampler.setName("Child Sampler");
+
+ FixedDelayTimer timer = new FixedDelayTimer(timerDelayMs);
+ timer.setName("Long Timer");
+ timer.setEnabled(true);
+
+ LoopController loop = new LoopController();
+ loop.setLoops(LoopController.INFINITE_LOOP_COUNT);
+ loop.setContinueForever(true);
+ loop.setEnabled(true);
+
+ ListedHashTree hashTree = new ListedHashTree();
+ hashTree.add(loop);
+ hashTree.add(loop, transactionController);
+ hashTree.add(transactionController, listener);
+ hashTree.add(transactionController, sampler);
+ hashTree.add(sampler, timer);
Review Comment:
Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixed issue where transaction sample elapsed time was incorrectly inflated when a thread was stopped [jmeter]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on code in PR #6770:
URL: https://github.com/apache/jmeter/pull/6770#discussion_r4106386983
##########
src/core/src/main/java/org/apache/jmeter/threads/JMeterThread.java:
##########
@@ -526,6 +526,12 @@ private SampleResult processSampler(Sampler current, Sampler parent, JMeterConte
&& transactionResult == null
&& transactionSampler != null
&& transactionPack != null) {
+ // Thread was stopped mid-transaction (e.g. during ramp-down).
Review Comment:
Fixed
##########
src/core/src/main/java/org/apache/jmeter/threads/JMeterThread.java:
##########
@@ -526,6 +526,12 @@ private SampleResult processSampler(Sampler current, Sampler parent, JMeterConte
&& transactionResult == null
&& transactionSampler != null
&& transactionPack != null) {
+ // Thread was stopped mid-transaction (e.g. during ramp-down).
+ // Ensure setTransactionDone() is called so that elapsed time and idle time
+ // are correctly computed (see GitHub issue #6496 / Bug 55816).
+ if (!transactionSampler.isTransactionDone()) {
Review Comment:
Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org