jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Felix Schumacher <felix.schumac...@internetallee.de>
Subject Re: Setting better defaults for HTTP request
Date Wed, 30 Dec 2020 15:00:06 GMT

Am 20.12.20 um 21:31 schrieb Philippe Mouawad:
> Hello,
>
> I tried implementing it by modifying in those 2 methods the default value
>
>    -
>    https://github.com/apache/jmeter/blob/master/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java#L838
>    -
>    https://github.com/apache/jmeter/blob/master/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java#L846
>
>
> But they are not taken into account, it looks like the method returns 0
> because in this code:
>
>     public int getPropertyAsInt(String key, int defaultValue) {
>         JMeterProperty jmp = getRawProperty(key);
> => jmp is not NullProperty nor null, so jmp.getIntValue() is called leading
> to 0
>         return jmp == null || jmp instanceof NullProperty ? defaultValue :
> jmp.getIntValue();
>     }
>
> Is this another bug surfacing ?

I think it is.

When the element gets cloned, all properties are copied (including the
empty ones), so we don't get a NullProperty, but an empty StringProperty.

I think it is safe to add

---
a/src/core/src/main/java/org/apache/jmeter/testelement/AbstractTestElement.java
+++
b/src/core/src/main/java/org/apache/jmeter/testelement/AbstractTestElement.java
@@ -27,6 +27,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import org.apache.commons.lang3.StringUtils;
 import org.apache.jmeter.gui.Searchable;
 import org.apache.jmeter.testelement.property.BooleanProperty;
 import org.apache.jmeter.testelement.property.CollectionProperty;
@@ -240,7 +241,10 @@ public abstract class AbstractTestElement
implements TestElement, Serializable,
     @Override
     public int getPropertyAsInt(String key, int defaultValue) {
         JMeterProperty jmp = getRawProperty(key);
-        return jmp == null || jmp instanceof NullProperty ?
defaultValue : jmp.getIntValue();
+        if (jmp == null || jmp instanceof NullProperty ||
StringUtils.isBlank(jmp.getStringValue())) {
+            return defaultValue;
+        }
+        return jmp.getIntValue();
     }

to handle that case.

Question here is, should we add such logic to the other special handlers
(boolean, etc.), too?

Felix

>
> Regards
>
> On Sun, Dec 20, 2020 at 7:14 PM Antonio Gomes Rodrigues <ra0077@gmail.com>
> wrote:
>
>> Hi
>>
>> +1 to me to put default timeout
>>
>> Le dim. 20 déc. 2020 à 18:44, Graham Russell <graham@ham1.co.uk> a écrit
:
>>
>>> I agree, setting those as defaults is much better than infinite and less
>>> concerning than 10s/60s.
>>>
>>> They probably won't do much to stop people complaining about JMeter
>> hanging
>>> on shutdown,
>>> was this lack of default timeout the root cause of those complaints or is
>>> there something else we can do with that issue?
>>>
>>> On Sun, 20 Dec 2020, 13:02 Philippe Mouawad, <philippe.mouawad@gmail.com
>>>
>>> wrote:
>>>
>>>> Hello,
>>>> Let’s do that
>>>>
>>>> Thanks for feedback
>>>>
>>>> On Sunday, December 20, 2020, Felix Schumacher <
>>>> felix.schumacher@internetallee.de> wrote:
>>>>
>>>>> Am Samstag, den 19.12.2020, 17:10 +0100 schrieb Philippe Mouawad:
>>>>>> Hello
>>>>>>
>>>>>> Currently we don't set neither connect nor read timeout which means
>>>>>> they
>>>>>> default to infinite.
>>>>>> I don't think those are good defaults and users frequently think
>>>>>> JMeter is
>>>>>> hanging.
>>>>>>
>>>>>> Shouldn't we set better defaults ?
>>>>>>
>>>>>> - Connect  to 10s
>>>>>> - Read to 60s
>>>>> I generally like the idea of setting a default timeout, as infinity
>> is
>>>>> a long time to wait for. The times are great, if you know, that
>>>>> timeouts are set, but what about the old settings, where the plan
>>>>> didn't take those into account?
>>>>>
>>>>> I think we can be a bit more generous on those timeouts, especially
>> for
>>>>> the read timeout. For a non-interactive site, those might be a bit
>>>>> short.
>>>>>
>>>>> In my firefox's about:config the values for connection and response
>>>>> timeout are 90 s and 300 s respectively. (I took
>>>>> network.http.connection-timeout and network.http.response.timeout)
>>>>>
>>>>> I think those values should be conservative enough for most of our
>>>>> users.
>>>>>
>>>>>
>>>>> Felix
>>>>>
>>>>>> WDYT ?
>>>>>> Thanks
>>>>>
>>>> --
>>>> Cordialement.
>>>> Philippe Mouawad.
>>>>
>

Mime
View raw message