On Sun, Nov 24, 2013 at 12:10 AM, sebb <[email protected]> wrote: > On 23 November 2013 22:59, Philippe Mouawad <[email protected]> > wrote: > > Hello sebb, > > My answers below. > > > > > > On Sat, Nov 23, 2013 at 6:17 PM, sebb <[email protected]> wrote: > > > >> On 23 November 2013 16:22, Philippe Mouawad <[email protected] > > > >> wrote: > >> > On Sat, Nov 23, 2013 at 4:41 PM, sebb <[email protected]> wrote: > >> > > >> >> On 23 November 2013 14:03, Philippe Mouawad < > [email protected] > >> > > >> >> wrote: > >> >> > Hello, > >> >> > What about introducing a property called: > >> >> > > >> >> > - transaction_controller.include_processing_and_timers > >> >> > > >> >> > > >> >> > It would allow users who have existing plans to stay with existing > >> >> > behaviour by setting: > >> >> > > >> >> > - transaction_controller.include_processing_and_timers=true > >> >> > > >> >> > > >> >> > And default value would be false. > >> >> > >> >> That would change the behaviour for existing tests. > >> >> > >> > No if user sets > >> transaction_controller.include_processing_and_timers=true, > >> > everything remains as currently. > >> > We add a Incompatible change in changes.xml and everything is fine. > >> > >> Yes, I understand that, but it still requires existing users to update > >> their installation. > >> Whereas if the default is true, only users who want the new behaviour > >> need to change their settings. > >> > > > > See https://issues.apache.org/bugzilla/show_bug.cgi?id=55816 attached > Test > > Plan. > > Not sure that's relevant here. Seems like an obvious bug to me if the > TC does not honour the setting of the include check-box. > > It was just to point to reported response times when Include is checked. In Test plan I simulate processing that takes more than 1 second, and you can see impact on reported response time.
> > If the default is true, do you find it OK that users need to update their > > installation to have the best behaviour from JMeter when using TC ? > > I don't agree that it is necessarily best behaviour to ignore timers. > I usually use (And I don't think I am alone) TC as a wrapper to samplers (not always, but whenever I have more than one sample in a business Transaction). If I don't uncheck Include... then it would mean I would be reporting response time degraded by Post Processors behaviour (which are always present for Response Assertions for example). > > > I don't. Default should be the best ones, and backward compatibility > should > > not be more important than having the right behaviour. > > Again, I don't agree that the proposed behaviour is more correct than > the current behaviour. > > > I find it already very good that we introduce a special property to keep > > old behaviour. > > Fine, but unless the old behaviour is clearly wrong - which I dispute > - the default should be to keep the old behaviour. > > > > >> > >> > > >> >> I'm not keen on changing the behaviour unless it is clearly > incorrect. > >> >> I don't think that is the case here. > >> >> > >> > It is clearly incorrect because it reports in overall response time of > >> > Transaction Controller the time taken by post processors (I agree if > it > >> > reported only timers it would be acceptable) . > >> > > >> > >> It's still the overall time for the TC children. > >> > > > > Yes but do you agree that a user only wants cumulated response time > > excluding any JMeter processing. > > > > > >> If PostProcessors are taking a significant time to run, then that is > >> something to be looked at separately. > >> And there may be no PostProcessors. > >> > >> > > > > > >> > > >> > > >> >> > >> >> > By the way this impact should leed us to change something we've > been > >> >> doing > >> >> > up till now which is not to write the value of a property if it is > >> equal > >> >> to > >> >> > default. > >> >> > If we had not done this up until now , we could have changed this > >> default > >> >> > with no impact . > >> >> > >> >> I don't think that is true. > >> >> > >> >> AFAICT it's possible for the JMX to have one default and the property > >> >> another. > >> >> However it would probably require the code to check whether the test > >> >> element is brand new or has been created from the JMX. > >> >> > >> > > >> > My proposition is to always write the value wether equals or not to > >> > default. > >> > >> Which makes the JMX files much larger. > >> Also when the JMX file is saved it will change; if users store their > >> JMX files in an SCM system, it will generate unnecessary changes. > >> > >> >> > >> >> It would also have to be carefully documented as we have not done > this > >> so > >> >> far. > >> >> > >> >> > Regards > >> >> > Philippe > >> >> > > >> >> > > >> >> > > >> >> > On Tue, Oct 15, 2013 at 4:46 PM, sebb <[email protected]> wrote: > >> >> > > >> >> >> On 14 October 2013 22:05, Philippe Mouawad < > >> [email protected]> > >> >> >> wrote: > >> >> >> > The problem is that if you change it , then automatically it > will > >> >> change > >> >> >> > behaviour of current scripts I think. > >> >> >> > >> >> >> Yes. > >> >> >> > >> >> >> > As when reloading existing script, > >> >> >> > getPropertyAsBoolean(INCLUDE_TIMERS, > >> >> DEFAULT_VALUE_FOR_INCLUDE_TIMERS); > >> >> >> > will return DEFAULT_VALUE_FOR_INCLUDE_TIMERS , which will be > false > >> >> while > >> >> >> in > >> >> >> > fact while its previous meaning when user saved his script was > >> true. > >> >> >> > >> >> >> The value of the default cannot be changed without affecting > existing > >> >> >> test plans. > >> >> >> > >> >> >> > See https://issues.apache.org/bugzilla/show_bug.cgi?id=55498 > >> >> >> > > >> >> >> > In my opinion this should be changed adding an Incompatible > change > >> >> but we > >> >> >> > would need to accept that users who really need it have to > modify > >> >> their > >> >> >> > script. > >> >> >> > > >> >> >> > +1 for this breaking change. > >> >> >> > > >> >> >> > Regards > >> >> >> > Philippe > >> >> >> > > >> >> >> > > >> >> >> > > >> >> >> > > >> >> >> > On Mon, Oct 14, 2013 at 10:56 PM, Milamber <[email protected] > > > >> >> wrote: > >> >> >> > > >> >> >> >> > >> >> >> >> Le 14/10/2013 21:47, [email protected] a ecrit : > >> >> >> >> > >> >> >> >> Author: pmouawad > >> >> >> >>> Date: Mon Oct 14 20:47:58 2013 > >> >> >> >>> New Revision: 1532085 > >> >> >> >>> > >> >> >> >>> URL: http://svn.apache.org/r1532085 > >> >> >> >>> Log: > >> >> >> >>> Add constant > >> >> >> >>> > >> >> >> >> > >> >> >> >> Perhaps, It would be great to transform this default value to a > >> >> property > >> >> >> >> for allow user to change this. > >> >> >> >> > >> >> >> >> > >> >> >> >> > >> >> >> >> > >> >> >> >> > >> >> >> >>> Modified: > >> >> >> >>> jmeter/trunk/src/core/org/**apache/jmeter/control/** > >> >> >> >>> TransactionController.java > >> >> >> >>> > >> >> >> >>> Modified: jmeter/trunk/src/core/org/**apache/jmeter/control/** > >> >> >> >>> TransactionController.java > >> >> >> >>> URL: > http://svn.apache.org/viewvc/**jmeter/trunk/src/core/org/** > >> >> >> >>> apache/jmeter/control/**TransactionController.java?** > >> >> >> >>> rev=1532085&r1=1532084&r2=**1532085&view=diff< > >> >> >> > >> >> > >> > http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/control/TransactionController.java?rev=1532085&r1=1532084&r2=1532085&view=diff > >> >> >> > > >> >> >> >>> > ==============================**==============================** > >> >> >> >>> ================== > >> >> >> >>> --- > >> >> >> > >> >> > >> > jmeter/trunk/src/core/org/**apache/jmeter/control/**TransactionController.java > >> >> >> >>> (original) > >> >> >> >>> +++ > >> >> >> > >> >> > >> > jmeter/trunk/src/core/org/**apache/jmeter/control/**TransactionController.java > >> >> >> >>> Mon Oct 14 20:47:58 2013 > >> >> >> >>> @@ -53,6 +53,8 @@ public class TransactionController exten > >> >> >> >>> private static final Logger log = > LoggingManager.** > >> >> >> >>> getLoggerForClass(); > >> >> >> >>> + private static final boolean > >> >> DEFAULT_VALUE_FOR_INCLUDE_**TIMERS > >> >> >> = > >> >> >> >>> true; // default true for compatibility > >> >> >> >>> + > >> >> >> >>> /** > >> >> >> >>> * Only used in parent Mode > >> >> >> >>> */ > >> >> >> >>> @@ -293,7 +295,7 @@ public class TransactionController exten > >> >> >> >>> * @param includeTimers > >> >> >> >>> */ > >> >> >> >>> public void setIncludeTimers(boolean includeTimers) { > >> >> >> >>> - setProperty(INCLUDE_TIMERS, includeTimers, true); // > >> >> default > >> >> >> >>> true for compatibility > >> >> >> >>> + setProperty(INCLUDE_TIMERS, includeTimers, > >> >> >> >>> DEFAULT_VALUE_FOR_INCLUDE_**TIMERS); > >> >> >> >>> } > >> >> >> >>> /** > >> >> >> >>> @@ -302,6 +304,6 @@ public class TransactionController exten > >> >> >> >>> * @return boolean (defaults to true for backwards > >> >> compatibility) > >> >> >> >>> */ > >> >> >> >>> public boolean isIncludeTimers() { > >> >> >> >>> - return getPropertyAsBoolean(INCLUDE_**TIMERS, true); > >> >> >> >>> + return getPropertyAsBoolean(INCLUDE_**TIMERS, > >> >> >> >>> DEFAULT_VALUE_FOR_INCLUDE_**TIMERS); > >> >> >> >>> } > >> >> >> >>> } > >> >> >> >>> > >> >> >> >>> > >> >> >> >>> > >> >> >> >>> > >> >> >> >> > >> >> >> > > >> >> >> > > >> >> >> > -- > >> >> >> > Cordialement. > >> >> >> > Philippe Mouawad. > >> >> >> > >> >> > > >> >> > > >> >> > > >> >> > -- > >> >> > Cordialement. > >> >> > Philippe Mouawad. > >> >> > >> > > >> > > >> > > >> > -- > >> > Cordialement. > >> > Philippe Mouawad. > >> > > > > > > > > -- > > Cordialement. > > Philippe Mouawad. > -- Cordialement. Philippe Mouawad.
