Re: svn commit: r1183065 - in /jakarta/jmeter/trunk: src/core/org/apache/jmeter/engine/ src/core/org/apache/jmeter/gui/action/ src/core/org/apache/jmeter/gui/util/ src/core/org/apache/jmeter/resources/ xdocs/

On 13 October 2011 21:12,  <[email protected]> wrote:
> Author: pmouawad
> Date: Thu Oct 13 20:12:23 2011
> New Revision: 1183065
>
> URL: http://svn.apache.org/viewvc?rev=3D1183065&view=3Drev
> Log:
> Bug 52019 - Add menu option to Start a test ignoring Pause Timers
>
> Added:
> =A0 =A0jakarta/jmeter/trunk/src/core/org/apache/jmeter/engine/TreeClonerN=
oTimer.java =A0 (with props)
> Modified:
> =A0 =A0jakarta/jmeter/trunk/src/core/org/apache/jmeter/engine/TreeCloner.=
java
> =A0 =A0jakarta/jmeter/trunk/src/core/org/apache/jmeter/gui/action/ActionN=
ames.java
> =A0 =A0jakarta/jmeter/trunk/src/core/org/apache/jmeter/gui/action/Start.j=
ava
> =A0 =A0jakarta/jmeter/trunk/src/core/org/apache/jmeter/gui/util/JMeterMen=
uBar.java
> =A0 =A0jakarta/jmeter/trunk/src/core/org/apache/jmeter/resources/messages=
.properties
> =A0 =A0jakarta/jmeter/trunk/src/core/org/apache/jmeter/resources/messages=
_es.properties
> =A0 =A0jakarta/jmeter/trunk/src/core/org/apache/jmeter/resources/messages=
_fr.properties
> =A0 =A0jakarta/jmeter/trunk/src/core/org/apache/jmeter/resources/messages=
_pt_BR.properties
> =A0 =A0jakarta/jmeter/trunk/xdocs/changes.xml
>
> Modified: jakarta/jmeter/trunk/src/core/org/apache/jmeter/engine/TreeClon=
er.java
> URL: http://svn.apache.org/viewvc/jakarta/jmeter/trunk/src/core/org/apach=
e/jmeter/engine/TreeCloner.java?rev=3D1183065&r1=3D1183064&r2=3D1183065&vie=
w=3Ddiff
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- jakarta/jmeter/trunk/src/core/org/apache/jmeter/engine/TreeCloner.jav=
a (original)
> +++ jakarta/jmeter/trunk/src/core/org/apache/jmeter/engine/TreeCloner.jav=
a Thu Oct 13 20:12:23 2011
> @@ -56,7 +56,6 @@ public class TreeCloner implements HashT
> =A0 =A0 }
>
> =A0 =A0 public void addNode(Object node, HashTree subTree) {
> -
> =A0 =A0 =A0 =A0 if ( (node instanceof TestElement) // Check can cast for =
clone
> =A0 =A0 =A0 =A0 =A0 =A0// Don't clone NoThreadClone unless honourNoThread=
Clone =3D=3D false
> =A0 =A0 =A0 =A0 =A0 && (!honourNoThreadClone || !(node instanceof NoThrea=
dClone))
> @@ -66,6 +65,14 @@ public class TreeCloner implements HashT
> =A0 =A0 =A0 =A0 } else {
> =A0 =A0 =A0 =A0 =A0 =A0 newTree.add(objects, node);
> =A0 =A0 =A0 =A0 }
> + =A0 =A0 =A0 =A0addLast(node);
> + =A0 =A0}
> +
> + =A0 =A0/**
> + =A0 =A0 * add node to objects LinkedList
> + =A0 =A0 * @param node Object
> + =A0 =A0 */
> + =A0 =A0protected final void addLast(Object node) {
> =A0 =A0 =A0 =A0 objects.addLast(node);
> =A0 =A0 }

OK, so subclasses can override this to skip adding a node.

>
> Added: jakarta/jmeter/trunk/src/core/org/apache/jmeter/engine/TreeClonerN=
oTimer.java
> URL: http://svn.apache.org/viewvc/jakarta/jmeter/trunk/src/core/org/apach=
e/jmeter/engine/TreeClonerNoTimer.java?rev=3D1183065&view=3Dauto
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- jakarta/jmeter/trunk/src/core/org/apache/jmeter/engine/TreeClonerNoTi=
mer.java (added)
> +++ jakarta/jmeter/trunk/src/core/org/apache/jmeter/engine/TreeClonerNoTi=
mer.java Thu Oct 13 20:12:23 2011
> @@ -0,0 +1,59 @@
> +/*
> + * Licensed to the Apache Software Foundation (ASF) under one or more
> + * contributor license agreements. =A0See the NOTICE file distributed wi=
th
> + * this work for additional information regarding copyright ownership.
> + * The ASF licenses this file to You under the Apache License, Version 2=
.0
> + * (the "License"); you may not use this file except in compliance with
> + * the License. =A0You may obtain a copy of the License at
> + *
> + * =A0 http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or impli=
ed.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + *
> + */
> +
> +package org.apache.jmeter.engine;
> +
> +import org.apache.jmeter.timers.Timer;
> +import org.apache.jorphan.collections.HashTree;
> +import org.apache.jorphan.logging.LoggingManager;
> +import org.apache.log.Logger;
> +
> +/**
> + * Clones the test tree, =A0skipping test elements that implement {@link=
 Timer} by default.
> + */
> +public class TreeClonerNoTimer extends TreeCloner{
> + =A0 =A0private Logger logger =3D LoggingManager.getLoggerForClass();
> +
> + =A0 =A0/**
> + =A0 =A0 * {@inheritDoc}
> + =A0 =A0 */
> + =A0 =A0public TreeClonerNoTimer() {
> + =A0 =A0 =A0 =A0super();
> + =A0 =A0}
> +
> + =A0 =A0/**
> + =A0 =A0 * {@inheritDoc}
> + =A0 =A0 */
> + =A0 =A0public TreeClonerNoTimer(boolean honourNoThreadClone) {
> + =A0 =A0 =A0 =A0super(honourNoThreadClone);
> + =A0 =A0}
> +
> + =A0 =A0/**
> + =A0 =A0 * {@inheritDoc}
> + =A0 =A0 */

@Override?

> + =A0 =A0public void addNode(Object node, HashTree subTree) {
> + =A0 =A0 =A0 =A0if(!(node instanceof Timer)) {

It's confusing to use negative conditions

> + =A0 =A0 =A0 =A0 =A0 =A0super.addNode(node, subTree);
> + =A0 =A0 =A0 =A0} else {
> + =A0 =A0 =A0 =A0 =A0 =A0if(logger.isDebugEnabled()) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0logger.debug("Ignoring timer node:"+ nod=
e);
> + =A0 =A0 =A0 =A0 =A0 =A0}
> + =A0 =A0 =A0 =A0 =A0 =A0addLast(node);

This looks wrong, surely you don't want to add the node?

> + =A0 =A0 =A0 =A0}
> + =A0 =A0}
> +}

However, the whole approach looks wrong.

Why not just override addLast(), and skip super.addLast() if processing a T=
imer?