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

Use JMeterProperty#intValue for loop count directly #5876

Merged
merged 2 commits into from
May 11, 2023

Conversation

FSchumacher
Copy link
Contributor

Description

This will get rid of the roundtrip over a String in case, the prop is already an IntegerProperty.

Motivation and Context

Came up in the discussion about #5875 and looks like an easy first fix.

How Has This Been Tested?

Should be covered by the unit tests already

Types of changes

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

Checklist:

This will get rid of the roundtrip over a String in case,
the prop is already an IntegerProperty.
@vlsi
Copy link
Collaborator

vlsi commented May 4, 2023

Could you please fix the other getStringValue->parseInt cases?
For instance: ThroughputController, JMSSampler.

@FSchumacher
Copy link
Contributor Author

I looked at ThroughputController and it is not easily fixed by using this approach, as it relies on a different default value.
The AbstractProperty#getFloatValue catches NumberFormatExceptions and returns 0. So, using it directly would lead to a different default value in case of a broken value.
How did you search for those occurrences? I had to look a bit longer to find line 600 in JMSSampler, but there are surely more? Line 600 is a good candidate to replace, so I will extend this PR with it. Feel free, to push any changes, too.

@vlsi
Copy link
Collaborator

vlsi commented May 5, 2023

I used "find usages" for getStringValue, and then I filtered the entries that parse the result as integer.

I think this looks like

String valueString = prop.getStringValue();
try {
retVal = Integer.parseInt(valueString);
is pretty much like "get property as int" to me.

I'm not sure what to do with getFloatValue, however, it looks like getStringValue + parseFloat should also be replaced with getFloatValue.

@FSchumacher
Copy link
Contributor Author

If you look at that method, you will see, that it assumes a default value of 1 and uses that in case of a NumberFormatException. If we use JMeterProperty#getIntValue we would never get that exception, but a default value of 0.

@vlsi vlsi added this to the 5.6 milestone May 11, 2023
@vlsi vlsi merged commit 96bfab9 into apache:master May 11, 2023
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.

2 participants