You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by "milamberspace (via GitHub)" <gi...@apache.org> on 2023/05/10 08:44:18 UTC
[GitHub] [jmeter] milamberspace opened a new pull request, #5901: Fix NumberFormatException when counter is empty or not a digit on Pro…
milamberspace opened a new pull request, #5901:
URL: https://github.com/apache/jmeter/pull/5901
…xy Settings panel
## Description
On proxy settings, if the Counter is empty or not a numeric value, when you click on Set Counter button, JMeter have a NumberFormatException
2023-05-04 17:10:42,819 ERROR o.a.j.JMeter: Uncaught exception in thread Thread[AWT-EventQueue-0,6,main]
java.lang.NumberFormatException: For input string: ""
at java.lang.NumberFormatException.forInputString(NumberFormatException.java:67) ~[?:?]
at java.lang.Integer.parseInt(Integer.java:678) ~[?:?]
at java.lang.Integer.parseInt(Integer.java:786) ~[?:?]
at org.apache.jmeter.protocol.http.proxy.gui.ProxyControlGui.lambda$createHTTPSamplerPanel$1(ProxyControlGui.java:985) ~[ApacheJMeter_http.jar:5.4.3]
## Motivation and Context
This PR fix this issue with a test on the value, and display a popup if not a digit value
## How Has This Been Tested?
* Enter text or leave empty the counter field and click on Set Counter button
## Screenshots (if appropriate):
## Types of changes
- Bug fix (non-breaking change which fixes an issue)
## Checklist:
- [ X] My code follows the [code style][style-guide] of this project.
- [ ] I have updated the documentation accordingly.
--
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
[GitHub] [jmeter] vlsi commented on a diff in pull request #5901: Fix NumberFormatException when counter is empty or not a digit on Proxy Settings panel
Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on code in PR #5901:
URL: https://github.com/apache/jmeter/pull/5901#discussion_r1189695822
##########
src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/proxy/gui/ProxyControlGui.java:
##########
@@ -691,6 +698,17 @@ void enableRestart() {
}
}
+ void setSetCounters() {
+ if ((counterValue.getText().length() > 0) && NumberUtils.isParsable(counterValue.getText())) {
+ Proxy.setCounter(Integer.parseInt(counterValue.getText()));
+ } else {
+ JOptionPane.showMessageDialog(this,
+ //"Counter isn't a number value.", "Error on counter value", JOptionPane.WARNING_MESSAGE);
+ JMeterUtils.getResString("proxy_settings_counter_error_digits"), // $NON-NLS-1$
+ JMeterUtils.getResString("proxy_settings_counter_error_invalid_data"), // $NON-NLS-1$
+ JOptionPane.WARNING_MESSAGE);
Review Comment:
Ok, I see the number was already parsed without evaluating the properties, so supporting `${..}` there would be a different issue
--
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
[GitHub] [jmeter] vlsi commented on a diff in pull request #5901: Fix NumberFormatException when counter is empty or not a digit on Pro…
Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on code in PR #5901:
URL: https://github.com/apache/jmeter/pull/5901#discussion_r1189685935
##########
src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/proxy/gui/ProxyControlGui.java:
##########
@@ -691,6 +698,17 @@ void enableRestart() {
}
}
+ void setSetCounters() {
+ if ((counterValue.getText().length() > 0) && NumberUtils.isParsable(counterValue.getText())) {
+ Proxy.setCounter(Integer.parseInt(counterValue.getText()));
+ } else {
+ JOptionPane.showMessageDialog(this,
+ //"Counter isn't a number value.", "Error on counter value", JOptionPane.WARNING_MESSAGE);
+ JMeterUtils.getResString("proxy_settings_counter_error_digits"), // $NON-NLS-1$
+ JMeterUtils.getResString("proxy_settings_counter_error_invalid_data"), // $NON-NLS-1$
+ JOptionPane.WARNING_MESSAGE);
Review Comment:
E.g. something like `${__P(staringValue)}`
--
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
[GitHub] [jmeter] vlsi commented on a diff in pull request #5901: Fix NumberFormatException when counter is empty or not a digit on Proxy Settings panel
Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on code in PR #5901:
URL: https://github.com/apache/jmeter/pull/5901#discussion_r1189707423
##########
src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/proxy/gui/ProxyControlGui.java:
##########
@@ -691,6 +698,17 @@ void enableRestart() {
}
}
+ void setSetCounters() {
+ if ((counterValue.getText().length() > 0) && NumberUtils.isParsable(counterValue.getText())) {
+ Proxy.setCounter(Integer.parseInt(counterValue.getText()));
+ } else {
+ JOptionPane.showMessageDialog(this,
+ //"Counter isn't a number value.", "Error on counter value", JOptionPane.WARNING_MESSAGE);
+ JMeterUtils.getResString("proxy_settings_counter_error_digits"), // $NON-NLS-1$
+ JMeterUtils.getResString("proxy_settings_counter_error_invalid_data"), // $NON-NLS-1$
+ JOptionPane.WARNING_MESSAGE);
Review Comment:
I have no use case for evaluation `${..}` there yet, however, I think having uniform support for `${..}` would be nice
--
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
[GitHub] [jmeter] milamberspace commented on a diff in pull request #5901: Fix NumberFormatException when counter is empty or not a digit on Pro…
Posted by "milamberspace (via GitHub)" <gi...@apache.org>.
milamberspace commented on code in PR #5901:
URL: https://github.com/apache/jmeter/pull/5901#discussion_r1189670497
##########
src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/proxy/gui/ProxyControlGui.java:
##########
@@ -691,6 +698,17 @@ void enableRestart() {
}
}
+ void setSetCounters() {
+ if ((counterValue.getText().length() > 0) && NumberUtils.isParsable(counterValue.getText())) {
+ Proxy.setCounter(Integer.parseInt(counterValue.getText()));
+ } else {
+ JOptionPane.showMessageDialog(this,
+ //"Counter isn't a number value.", "Error on counter value", JOptionPane.WARNING_MESSAGE);
+ JMeterUtils.getResString("proxy_settings_counter_error_digits"), // $NON-NLS-1$
+ JMeterUtils.getResString("proxy_settings_counter_error_invalid_data"), // $NON-NLS-1$
+ JOptionPane.WARNING_MESSAGE);
Review Comment:
@vlsi what is ${...} expressions? I don't understand
Not sure that add the problematic value is needed, specially if the value is empty. IMO only display a warning is ok.
--
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
[GitHub] [jmeter] vlsi commented on a diff in pull request #5901: Fix NumberFormatException when counter is empty or not a digit on Pro…
Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on code in PR #5901:
URL: https://github.com/apache/jmeter/pull/5901#discussion_r1189586434
##########
src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/proxy/gui/ProxyControlGui.java:
##########
@@ -691,6 +698,17 @@ void enableRestart() {
}
}
+ void setSetCounters() {
+ if ((counterValue.getText().length() > 0) && NumberUtils.isParsable(counterValue.getText())) {
+ Proxy.setCounter(Integer.parseInt(counterValue.getText()));
+ } else {
+ JOptionPane.showMessageDialog(this,
+ //"Counter isn't a number value.", "Error on counter value", JOptionPane.WARNING_MESSAGE);
+ JMeterUtils.getResString("proxy_settings_counter_error_digits"), // $NON-NLS-1$
+ JMeterUtils.getResString("proxy_settings_counter_error_invalid_data"), // $NON-NLS-1$
+ JOptionPane.WARNING_MESSAGE);
Review Comment:
Could you add the problematic value to the error message?
--
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
[GitHub] [jmeter] asfgit merged pull request #5901: Fix NumberFormatException when counter is empty or not a digit on Proxy Settings panel
Posted by "asfgit (via GitHub)" <gi...@apache.org>.
asfgit merged PR #5901:
URL: https://github.com/apache/jmeter/pull/5901
--
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
[GitHub] [jmeter] vlsi commented on a diff in pull request #5901: Fix NumberFormatException when counter is empty or not a digit on Pro…
Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on code in PR #5901:
URL: https://github.com/apache/jmeter/pull/5901#discussion_r1189588775
##########
src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/proxy/gui/ProxyControlGui.java:
##########
@@ -691,6 +698,17 @@ void enableRestart() {
}
}
+ void setSetCounters() {
+ if ((counterValue.getText().length() > 0) && NumberUtils.isParsable(counterValue.getText())) {
+ Proxy.setCounter(Integer.parseInt(counterValue.getText()));
+ } else {
+ JOptionPane.showMessageDialog(this,
+ //"Counter isn't a number value.", "Error on counter value", JOptionPane.WARNING_MESSAGE);
+ JMeterUtils.getResString("proxy_settings_counter_error_digits"), // $NON-NLS-1$
+ JMeterUtils.getResString("proxy_settings_counter_error_invalid_data"), // $NON-NLS-1$
+ JOptionPane.WARNING_MESSAGE);
Review Comment:
Will it support `${...}` expressions?
--
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
[GitHub] [jmeter] vlsi commented on a diff in pull request #5901: Fix NumberFormatException when counter is empty or not a digit on Proxy Settings panel
Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on code in PR #5901:
URL: https://github.com/apache/jmeter/pull/5901#discussion_r1189718034
##########
src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/proxy/gui/ProxyControlGui.java:
##########
@@ -691,6 +698,17 @@ void enableRestart() {
}
}
+ void setSetCounters() {
+ if ((counterValue.getText().length() > 0) && NumberUtils.isParsable(counterValue.getText())) {
+ Proxy.setCounter(Integer.parseInt(counterValue.getText()));
+ } else {
+ JOptionPane.showMessageDialog(this,
+ //"Counter isn't a number value.", "Error on counter value", JOptionPane.WARNING_MESSAGE);
+ JMeterUtils.getResString("proxy_settings_counter_error_digits"), // $NON-NLS-1$
+ JMeterUtils.getResString("proxy_settings_counter_error_invalid_data"), // $NON-NLS-1$
+ JOptionPane.WARNING_MESSAGE);
Review Comment:
That works for me. However, would you please include the problematic value itself in the error message?
--
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
[GitHub] [jmeter] milamberspace commented on a diff in pull request #5901: Fix NumberFormatException when counter is empty or not a digit on Proxy Settings panel
Posted by "milamberspace (via GitHub)" <gi...@apache.org>.
milamberspace commented on code in PR #5901:
URL: https://github.com/apache/jmeter/pull/5901#discussion_r1189702353
##########
src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/proxy/gui/ProxyControlGui.java:
##########
@@ -691,6 +698,17 @@ void enableRestart() {
}
}
+ void setSetCounters() {
+ if ((counterValue.getText().length() > 0) && NumberUtils.isParsable(counterValue.getText())) {
+ Proxy.setCounter(Integer.parseInt(counterValue.getText()));
+ } else {
+ JOptionPane.showMessageDialog(this,
+ //"Counter isn't a number value.", "Error on counter value", JOptionPane.WARNING_MESSAGE);
+ JMeterUtils.getResString("proxy_settings_counter_error_digits"), // $NON-NLS-1$
+ JMeterUtils.getResString("proxy_settings_counter_error_invalid_data"), // $NON-NLS-1$
+ JOptionPane.WARNING_MESSAGE);
Review Comment:
Seems usual to not evaluate the value against the JMeter function for the Proxy settings. Proxy is use only on GUI mode, not in load tests, so it's not need to evaluate the value if ${...} expression is present. Are you ok with this?
--
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
[GitHub] [jmeter] milamberspace commented on a diff in pull request #5901: Fix NumberFormatException when counter is empty or not a digit on Proxy Settings panel
Posted by "milamberspace (via GitHub)" <gi...@apache.org>.
milamberspace commented on code in PR #5901:
URL: https://github.com/apache/jmeter/pull/5901#discussion_r1189715944
##########
src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/proxy/gui/ProxyControlGui.java:
##########
@@ -691,6 +698,17 @@ void enableRestart() {
}
}
+ void setSetCounters() {
+ if ((counterValue.getText().length() > 0) && NumberUtils.isParsable(counterValue.getText())) {
+ Proxy.setCounter(Integer.parseInt(counterValue.getText()));
+ } else {
+ JOptionPane.showMessageDialog(this,
+ //"Counter isn't a number value.", "Error on counter value", JOptionPane.WARNING_MESSAGE);
+ JMeterUtils.getResString("proxy_settings_counter_error_digits"), // $NON-NLS-1$
+ JMeterUtils.getResString("proxy_settings_counter_error_invalid_data"), // $NON-NLS-1$
+ JOptionPane.WARNING_MESSAGE);
Review Comment:
This PR aims first to correct the NFE (which is a technical error), and not to make improvements in the Counter field. If there is a need to add functionality for evaluating JMeter functions, that will be the subject of another ticket. Can I have your LGTM?
--
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