You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@jmeter.apache.org by bu...@apache.org on 2014/12/21 15:57:27 UTC
[Bug 57381] New: HTTP(S) Test Script Recorder should display an
error if no Recording Controller is setup instead of recording in incorrect
places
https://issues.apache.org/bugzilla/show_bug.cgi?id=57381
Bug ID: 57381
Summary: HTTP(S) Test Script Recorder should display an error
if no Recording Controller is setup instead of
recording in incorrect places
Product: JMeter
Version: 2.12
Hardware: All
OS: All
Status: NEW
Severity: enhancement
Priority: P2
Component: HTTP
Assignee: issues@jmeter.apache.org
Reporter: p.mouawad@ubik-ingenierie.com
Currently, HTTP(S) Test Script Recorder is very permissive when no recording
controller is setup.
Documentation only mentions Recording Controller possibility but implementation
tries to find a node in the following order:
- Defined / Saved target
- Recording Controller
- Thread Group
- Workbench
This behaviour is disturbing for many reasons:
- Created samples can sometime appear where you do not expect them to
- They can be created in Workbench which is weird as they will not be really
usable
I propose to change this behaviour to:
- Display an error popup on start of proxy if targetNode.getTestElement() is
not of type : GenericController or AbstractThreadGroup.
In fact I think it should be only Recording Controller but this may break
existing behaviour.
Thoughts ?
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 57381] HTTP(S) Test Script Recorder should display an error if
Target Controller references a Recording Controller and no Recording
Controller exists
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57381
--- Comment #15 from Sebb <se...@apache.org> ---
(In reply to UbikLoadPack support from comment #14)
> (In reply to Sebb from comment #13)
> > (In reply to UbikLoadPack support from comment #12)
> >
> > > Ok, note that as users we would prefer "Use Recording Controller" to stay.
> >
> > Why is it still necessary (assuming 57832 is fixed)?
> >
> > It seems wrong to me to have a drop-down entry which refers to a
> > non-existent controller.
>
> It is more of a conceptual Target saying Use Recording controller located
> somewhere in test plan. By implementing the popup check it would allow to
> leave this feature retaining backward compatibility.
If I understand the pop-up proposal correctly, it would prevent the user from
running the test plan. So it would not retain compatibility.
However maybe the intention is for the pop-up to be a Warning (rather than an
Error).
But I'm not sure how useful it is to retain the existing behaviour, as it is
rather unexpected.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 57381] HTTP(S) Test Script Recorder should display an error if
Target Controller references a Recording Controller and no Recording
Controller exists
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57381
Philippe Mouawad <p....@ubik-ingenierie.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Keywords| |PatchAvailable
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 57381] HTTP(S) Test Script Recorder should display an error if
Target Controller references a Recording Controller and no Recording
Controller exists
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57381
--- Comment #1 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
(In reply to Philippe Mouawad from comment #0)
> Currently, HTTP(S) Test Script Recorder is very permissive when no recording
> controller is setup.
> Documentation only mentions Recording Controller possibility but
> implementation tries to find a node in the following order:
> - Defined / Saved target
> - Recording Controller
> - Thread Group
> - Workbench
>
> This behaviour is disturbing for many reasons:
> - Created samples can sometime appear where you do not expect them to
> - They can be created in Workbench which is weird as they will not be really
> usable
>
> I propose to change this behaviour to:
> - Display an error popup on start of proxy if targetNode.getTestElement() is
> not of type : GenericController or AbstractThreadGroup.
>
> In fact I think it should be only Recording Controller but this may break
> existing behaviour.
>
> Thoughts ?
I propose to change this behaviour to:
- Display an error popup on start of proxy if "Target Controller" is configured
to "Recording Controller" and if no "Recording Controller" exists
Thoughts
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 57381] HTTP(S) Test Script Recorder should display an error if
Target Controller references a Recording Controller and no Recording
Controller exists
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57381
UbikLoadPack support <su...@ubikloadpack.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |support@ubikloadpack.com
--- Comment #16 from UbikLoadPack support <su...@ubikloadpack.com> ---
Created attachment 32330
--> https://issues.apache.org/bugzilla/attachment.cgi?id=32330&action=edit
Patch implementing feature
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 57381] HTTP(S) Test Script Recorder should display an error if
Target Controller references a Recording Controller and no Recording
Controller exists
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57381
--- Comment #14 from UbikLoadPack support <su...@ubikloadpack.com> ---
(In reply to Sebb from comment #13)
> (In reply to UbikLoadPack support from comment #12)
>
> > Ok, note that as users we would prefer "Use Recording Controller" to stay.
>
> Why is it still necessary (assuming 57832 is fixed)?
>
> It seems wrong to me to have a drop-down entry which refers to a
> non-existent controller.
It is more of a conceptual Target saying Use Recording controller located
somewhere in test plan. By implementing the popup check it would allow to leave
this feature retaining backward compatibility.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 57381] HTTP(S) Test Script Recorder should display an error if
Target Controller references a Recording Controller and no Recording
Controller exists
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57381
--- Comment #12 from UbikLoadPack support <su...@ubikloadpack.com> ---
(In reply to Sebb from comment #11)
> (In reply to UbikLoadPack support from comment #10)
> > (In reply to Sebb from comment #9)
> > > (In reply to UbikLoadPack support from comment #7)
> > > > (In reply to Dzmitry Kashlach from comment #3)
> > > > > Hi JMeter team,
> > > > >
> > > > > May be, another IMHO good variant is to remove "Use recording controller"
> > > > > from drop-down list e.g. to show user only nodes, which are really present
> > > > > in test-tree(Thread Groups, Controllers, whatever else)?
> > > >
> > > > Hi, another argument why removing it is not a good idea IMHO is bug 57382.
> > > > If you remove it then when reloading you will need to setup correctly target
> > > > before using Test Plan, while with "Use recording controller", nothing is
> > > > needed if you have a Recording controller.
> > >
> > > Not sure I see how Bug 57832 affects this.
> >
> > Well if you remove "Use Recording Controller", then it means you rely on
> > user setting the target controller explicitely right ?
>
> Yes.
>
> > So if he does, he saves his plans and reloads it later, due to bug the
> > setting is forgotten and it now records not where user wanted it to record
> > but as per the result of ProxyControl#findTargetControllerNode
> >
> > So you have lost productivity with this change.
>
> Not if bug 57832 has been fixed, surely?
>
> > Regarding "Cannot the the recording template can be fixed so that it just
> > works?" ? No you need to fix 57832 before.
>
> Fine, let's fix 57832.
>
> > Regards
Ok, note that as users we would prefer "Use Recording Controller" to stay.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 57381] HTTP(S) Test Script Recorder should display an error if
Target Controller references a Recording Controller and no Recording
Controller exists
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57381
--- Comment #18 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
Thanks for patch
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 57381] HTTP(S) Test Script Recorder should display an error if
Target Controller references a Recording Controller and no Recording
Controller exists
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57381
--- Comment #10 from UbikLoadPack support <su...@ubikloadpack.com> ---
(In reply to Sebb from comment #9)
> (In reply to UbikLoadPack support from comment #7)
> > (In reply to Dzmitry Kashlach from comment #3)
> > > Hi JMeter team,
> > >
> > > May be, another IMHO good variant is to remove "Use recording controller"
> > > from drop-down list e.g. to show user only nodes, which are really present
> > > in test-tree(Thread Groups, Controllers, whatever else)?
> >
> > Hi, another argument why removing it is not a good idea IMHO is bug 57382.
> > If you remove it then when reloading you will need to setup correctly target
> > before using Test Plan, while with "Use recording controller", nothing is
> > needed if you have a Recording controller.
>
> Not sure I see how Bug 57832 affects this.
Well if you remove "Use Recording Controller", then it means you rely on user
setting the target controller explicitely right ?
So if he does, he saves his plans and reloads it later, due to bug the setting
is forgotten and it now records not where user wanted it to record but as per
the result of ProxyControl#findTargetControllerNode
So you have lost productivity with this change.
Regarding "Cannot the the recording template can be fixed so that it just
works?" ? No you need to fix 57832 before.
Regards
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 57381] HTTP(S) Test Script Recorder should display an error if
Target Controller references a Recording Controller and no Recording
Controller exists
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57381
--- Comment #3 from Dzmitry Kashlach <dz...@blazemeter.com> ---
Hi JMeter team,
May be, another IMHO good variant is to remove "Use recording controller" from
drop-down list e.g. to show user only nodes, which are really present in
test-tree(Thread Groups, Controllers, whatever else)?
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 57381] HTTP(S) Test Script Recorder should display an error if
Target Controller references a Recording Controller and no Recording
Controller exists
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57381
Philippe Mouawad <p....@ubik-ingenierie.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |p.mouawad@ubik-ingenierie.c
| |om
Summary|HTTP(S) Test Script |HTTP(S) Test Script
|Recorder should display an |Recorder should display an
|error if no Recording |error if Target Controller
|Controller is setup instead |references a Recording
|of recording in incorrect |Controller and no Recording
|places |Controller exists
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 57381] HTTP(S) Test Script Recorder should display an error if
Target Controller references a Recording Controller and no Recording
Controller exists
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57381
Philippe Mouawad <p....@ubik-ingenierie.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Blocks| |25430
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 57381] HTTP(S) Test Script Recorder should display an error if
Target Controller references a Recording Controller and no Recording
Controller exists
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57381
--- Comment #19 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
Date: Sat Dec 27 16:01:49 2014
New Revision: 1648060
URL: http://svn.apache.org/r1648060
Log:
Bug 57381 - HTTP(S) Test Script Recorder should display an error if Target
Controller references a Recording Controller and no Recording Controller exists
Oups forgot i18n
Bugzilla Id: 57381
Modified:
jmeter/trunk/src/core/org/apache/jmeter/resources/messages.properties
jmeter/trunk/src/core/org/apache/jmeter/resources/messages_fr.properties
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 57381] HTTP(S) Test Script Recorder should display an error if
Target Controller references a Recording Controller and no Recording
Controller exists
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57381
--- Comment #11 from Sebb <se...@apache.org> ---
(In reply to UbikLoadPack support from comment #10)
> (In reply to Sebb from comment #9)
> > (In reply to UbikLoadPack support from comment #7)
> > > (In reply to Dzmitry Kashlach from comment #3)
> > > > Hi JMeter team,
> > > >
> > > > May be, another IMHO good variant is to remove "Use recording controller"
> > > > from drop-down list e.g. to show user only nodes, which are really present
> > > > in test-tree(Thread Groups, Controllers, whatever else)?
> > >
> > > Hi, another argument why removing it is not a good idea IMHO is bug 57382.
> > > If you remove it then when reloading you will need to setup correctly target
> > > before using Test Plan, while with "Use recording controller", nothing is
> > > needed if you have a Recording controller.
> >
> > Not sure I see how Bug 57832 affects this.
>
> Well if you remove "Use Recording Controller", then it means you rely on
> user setting the target controller explicitely right ?
Yes.
> So if he does, he saves his plans and reloads it later, due to bug the
> setting is forgotten and it now records not where user wanted it to record
> but as per the result of ProxyControl#findTargetControllerNode
>
> So you have lost productivity with this change.
Not if bug 57832 has been fixed, surely?
> Regarding "Cannot the the recording template can be fixed so that it just
> works?" ? No you need to fix 57832 before.
Fine, let's fix 57832.
> Regards
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 57381] HTTP(S) Test Script Recorder should display an error if
Target Controller references a Recording Controller and no Recording
Controller exists
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57381
--- Comment #4 from Sebb <se...@apache.org> ---
(In reply to Dzmitry Kashlach from comment #3)
> Hi JMeter team,
>
> May be, another IMHO good variant is to remove "Use recording controller"
> from drop-down list e.g. to show user only nodes, which are really present
> in test-tree(Thread Groups, Controllers, whatever else)?
+1
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 57381] HTTP(S) Test Script Recorder should display an error if
Target Controller references a Recording Controller and no Recording
Controller exists
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57381
--- Comment #9 from Sebb <se...@apache.org> ---
(In reply to UbikLoadPack support from comment #7)
> (In reply to Dzmitry Kashlach from comment #3)
> > Hi JMeter team,
> >
> > May be, another IMHO good variant is to remove "Use recording controller"
> > from drop-down list e.g. to show user only nodes, which are really present
> > in test-tree(Thread Groups, Controllers, whatever else)?
>
> Hi, another argument why removing it is not a good idea IMHO is bug 57382.
> If you remove it then when reloading you will need to setup correctly target
> before using Test Plan, while with "Use recording controller", nothing is
> needed if you have a Recording controller.
Not sure I see how Bug 57832 affects this.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 57381] HTTP(S) Test Script Recorder should display an error if
Target Controller references a Recording Controller and no Recording
Controller exists
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57381
--- Comment #8 from Sebb <se...@apache.org> ---
(In reply to UbikLoadPack support from comment #5)
> -1
> This method is really helpful specially if you use Recording template.
> Checking popup seems enough.
>
> Regards
Cannot the the recording template can be fixed so that it just works?
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 57381] HTTP(S) Test Script Recorder should display an error if
Target Controller references a Recording Controller and no Recording
Controller exists
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57381
--- Comment #6 from UbikLoadPack support <su...@ubikloadpack.com> ---
(In reply to Milamber from comment #2)
>
> +1
> The popup is a good idea.
+1 for this one
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 57381] HTTP(S) Test Script Recorder should display an error if
Target Controller references a Recording Controller and no Recording
Controller exists
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57381
--- Comment #13 from Sebb <se...@apache.org> ---
(In reply to UbikLoadPack support from comment #12)
> Ok, note that as users we would prefer "Use Recording Controller" to stay.
Why is it still necessary (assuming 57832 is fixed)?
It seems wrong to me to have a drop-down entry which refers to a non-existent
controller.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 57381] HTTP(S) Test Script Recorder should display an error if
Target Controller references a Recording Controller and no Recording
Controller exists
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57381
--- Comment #5 from UbikLoadPack support <su...@ubikloadpack.com> ---
-1
This method is really helpful specially if you use Recording template.
Checking popup seems enough.
Regards
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 57381] HTTP(S) Test Script Recorder should display an error if
Target Controller references a Recording Controller and no Recording
Controller exists
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57381
Philippe Mouawad <p....@ubik-ingenierie.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |FIXED
--- Comment #17 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
Date: Sat Dec 27 15:22:01 2014
New Revision: 1648056
URL: http://svn.apache.org/r1648056
Log:
Bug 57381 - HTTP(S) Test Script Recorder should display an error if Target
Controller references a Recording Controller and no Recording Controller exists
Bugzilla Id: 57381
Modified:
jmeter/trunk/src/components/org/apache/jmeter/control/gui/TreeNodeWrapper.java
jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/proxy/ProxyControl.java
jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/proxy/gui/ProxyControlGui.java
jmeter/trunk/xdocs/changes.xml
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 57381] HTTP(S) Test Script Recorder should display an error if
Target Controller references a Recording Controller and no Recording
Controller exists
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57381
--- Comment #7 from UbikLoadPack support <su...@ubikloadpack.com> ---
(In reply to Dzmitry Kashlach from comment #3)
> Hi JMeter team,
>
> May be, another IMHO good variant is to remove "Use recording controller"
> from drop-down list e.g. to show user only nodes, which are really present
> in test-tree(Thread Groups, Controllers, whatever else)?
Hi, another argument why removing it is not a good idea IMHO is bug 57382.
If you remove it then when reloading you will need to setup correctly target
before using Test Plan, while with "Use recording controller", nothing is
needed if you have a Recording controller.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 57381] HTTP(S) Test Script Recorder should display an error if
Target Controller references a Recording Controller and no Recording
Controller exists
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=57381
--- Comment #2 from Milamber <mi...@apache.org> ---
+1
The popup is a good idea.
--
You are receiving this mail because:
You are the assignee for the bug.