jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: svn commit: r1630232 - /jmeter/trunk/src/components/org/apache/jmeter/timers/SyncTimer.java
Date Thu, 09 Oct 2014 19:39:46 GMT
On 9 October 2014 20:27, Philippe Mouawad <philippe.mouawad@gmail.com> wrote:
> On Thu, Oct 9, 2014 at 3:32 AM, sebb <sebbaz@gmail.com> wrote:
>
>> On 8 October 2014 22:01,  <pmouawad@apache.org> 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.

> Feel free to find a better way.
>
>> >                  }
>> >              } catch (InterruptedException e) {
>> >                  return 0;
>> >
>> >
>>
>
>
>
> --
> Cordialement.
> Philippe Mouawad.

Mime
View raw message