On 9 October 2014 20:39, sebb <[email protected]> wrote:
> On 9 October 2014 20:27, Philippe Mouawad <[email protected]> wrote:
>> On Thu, Oct 9, 2014 at 3:32 AM, sebb <[email protected]> wrote:
>>
>>> On 8 October 2014 22:01, <[email protected]> wrote:
>>> > Author: pmouawad
>>> > Date: Wed Oct 8 21:01:52 2014
>>> > New Revision: 1630232
>>> >
>>> > URL: http://svn.apache.org/r1630232
>>> > Log:
>>> > Throw when configuration error (related to 57068)
>>> >
>>> > Modified:
>>> > jmeter/trunk/src/components/org/apache/jmeter/timers/SyncTimer.java
>>> >
>>> > Modified:
>>> jmeter/trunk/src/components/org/apache/jmeter/timers/SyncTimer.java
>>> > URL:
>>> http://svn.apache.org/viewvc/jmeter/trunk/src/components/org/apache/jmeter/timers/SyncTimer.java?rev=1630232&r1=1630231&r2=1630232&view=diff
>>> >
>>> ==============================================================================
>>> > --- jmeter/trunk/src/components/org/apache/jmeter/timers/SyncTimer.java
>>> (original)
>>> > +++ jmeter/trunk/src/components/org/apache/jmeter/timers/SyncTimer.java
>>> Wed Oct 8 21:01:52 2014
>>> > @@ -159,8 +159,10 @@ public class SyncTimer extends AbstractT
>>> > try {
>>> > if(timeoutInMs==0) {
>>> > arrival = this.barrier.await();
>>> > - } else {
>>> > + } else if(timeoutInMs > 0){
>>> > arrival = this.barrier.await(timeoutInMs,
>>> TimeUnit.MILLISECONDS);
>>> > + } else {
>>> > + throw new IllegalArgumentException("Negative value
>>> for timeout:"+timeoutInMs+" in Synchronizing Timer "+getName());
>>>
>>> -1
>>>
>>> This is the wrong place to check the value. It should be done in the
>>> GUI and/or the setter method, not at run-time.
>>>
>>> Setter does not seem to be a right place for me, setter should only be a
>> stupid method
>> Gui won't catch existing misconfiguration.
>
> But that does not mean we should allow the GUI to set a negative number.
>
>> The performance impact is low considering it should only happen in wrong
>> case.
>
> The performance impact is not at issue here.
> The point is that the code should try and catch the bug as early as possible.
>
> It's hard work trying to link an error in the log file to a GUI field.
There's another aspect to this - why should the incorrect value cause
the test to fail?
Would it not be better to just log the error and continue, especially
if the GUI does not provide validation?
>> Feel free to find a better way.
>>
>>> > }
>>> > } catch (InterruptedException e) {
>>> > return 0;
>>> >
>>> >
>>>
>>
>>
>>
>> --
>> Cordialement.
>> Philippe Mouawad.
