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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
