Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for control flow behavior regression #606

Merged
merged 1 commit into from
Jul 25, 2020
Merged

Conversation

tilln
Copy link
Contributor

@tilln tilln commented Jul 23, 2020

Description

Programmatic manipulation of the control flow via API methods of JMeterContext is not working as it used to.

When JMeter v5.0 introduced the "ability to Switch to next iteration of Current Loop" (see Bug 62238) the changes to JMeterThread were not entirely logically equivalent and broke some of our existing test when migrating to v5.x.

The simplest test plan to demonstrate this is as follows, where the Debug Sampler should never be run.
(This works up to JMeter v4.0 and fails in v5.0 and above.)

Test Plan
    \ Thread Group
        \ JSR223 Sampler
            ctx.setStartNextThreadLoop(true)
        \ Debug Sampler

Reason is the following change (refer here)

When processing the sample result, the "logical action" would only be changed if the result carried the "restart flag":

if(result.isStartNextThreadLoop()) {	            
    threadContext.setStartNextThreadLoop(true);	
}

But now the result unconditionally overwrites the context's "logical action":

threadContext.setTestLogicalAction(result.getTestLogicalAction());

This PR proposes to make the action change conditional again: only if different than CONTINUE

if (result.getTestLogicalAction() != TestLogicalAction.CONTINUE) {
    threadContext.setTestLogicalAction(result.getTestLogicalAction());
}

Motivation and Context

This fixes a breaking change from JMeter v4.0 to v5.0 that was supposed to be non-breaking.
Workarounds involve changing existing test plans by for example using
prev.setStartNextThreadLoop(true) (or the non-deprecated equivalent)
but this does not work everywhere e.g. in Preprocessors where there is no SampleResult yet.

How Has This Been Tested?

Has not yet been tested due to the trivial nature of the change.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the [code style][style-guide] of this project.
  • I have updated the documentation accordingly.

@vlsi
Copy link
Collaborator

vlsi commented Jul 23, 2020

Could you please add a test to validate the change?

@pmouawad pmouawad merged commit c0289a2 into apache:master Jul 25, 2020
ham1 pushed a commit to ham1/jmeter that referenced this pull request Sep 13, 2020
kkalinin pushed a commit to kkalinin/jmeter that referenced this pull request Mar 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants