[GitHub] jmeter pull request #364: let GUI component (dynamically) decide whether it ...

[ham1]

Github user ham1 commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/364#discussion_r158429468
  
--- Diff: src/core/org/apache/jmeter/gui/util/MenuInfo.java ---
@@ -74,4 +75,16 @@ public String getClassName(){
 public int getSortOrder() {
 return sortOrder;
 }
+
+/**
+ * Returns whether the menu item represented by this MenuInfo object 
should be enabled
+ * @param actionCommandthe action command name for the menu item
+ * @return true when menu item should be enabled, false otherwise.
+ */
+public boolean getEnabled(String actionCommand) {
+if (ActionNames.ADD.equals(actionCommand))
--- End diff --

Please use braces. Checkstyle should pick this up.


---

[GitHub] jmeter pull request #364: let GUI component (dynamically) decide whether it ...

[ptrd]

GitHub user ptrd opened a pull request:

https://github.com/apache/jmeter/pull/364

let GUI component (dynamically) decide whether it can be added via th…

…e menu or not; the menu item will be enabled/disabled accordingly.

## Description
Added the ability to have menu items for GUI components dynamically 
disabled/enabled.
The question whether a specific test element can be added (to the testplan) 
is delegated to the corresponding GUI component.

## Motivation and Context
In my websocket plugin 
(https://bitbucket.org/pjtr/jmeter-websocket-samplers), i have a config element 
that enables the user to configure specific (advanced) properties of the plugin 
(previously, this was done via JMeter properties, which is of course 
cumbersome). To avoid funny behaviour in the plugin and to avoid confusion of 
the user, such a config element should only be added once.

## How Has This Been Tested?
I tested it with my websocket plugin, see 
https://bitbucket.org/pjtr/jmeter-websocket-samplers/commits/branch/limit-number-of-advanced-options-element

## Screenshots (if appropriate):

## Types of changes

- New feature (non-breaking change which adds functionality)

## Checklist:


- [ x] My code follows the [code style][style-guide] of this project.
- [ ] I have updated the documentation accordingly.

[style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/ptrd/jmeter dynamic-add-menu

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/jmeter/pull/364.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #364


commit 72fccd5a6771565ffade43cce41763c3bc2df52f
Author: Peter Doornbosch 
Date:   2017-12-21T21:00:21Z

let GUI component (dynamically) decide whether it can be added via the menu 
or not; the menu item will be enabled/disabled accordingly.




---

Build failed in Jenkins: JMeter-trunk #6552

[Apache Jenkins Server]

See 


Changes:

[pmouawad] Add Apache License Header

[pmouawad] Bug 61919 - UX : Reorder Menus
Update tests
Contributed by Graham Russell
This closes #363

Bugzilla Id: 61919

[pmouawad] Bug 61919 - UX : Reorder Menus
Fix regressions : 
Unable to add Controllers to Thread Group
Unable to add Controllers to Controllers
Bugzilla Id: 61919

[pmouawad] Bug 61919 - UX : Reorder Menus
Reorder Config Elements
Bugzilla Id: 61919

[pmouawad] Bug 61919 - UX : Reorder Menus
Add order to Controllers
Bugzilla Id: 61919

[pmouawad] Bug 61919 - UX : Reorder Menus
Add order to Controllers
Bugzilla Id: 61919

[pmouawad] Fix SONAR bugs

[pmouawad] Reuse constructor
Indicate silent catch is voluntary

[pmouawad] Fix SONAR bugs

--
[...truncated 147.61 KB...]
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.http.config...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.http.config.gui...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.http.control...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.http.control.gui...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.http.gui...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.http.modifier...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.http.modifier.gui...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.http.parser...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.http.proxy...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.http.proxy.gui...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.http.sampler...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.http.sampler.hc...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.http.util...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.http.util.accesslog...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.http.visualizers...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.ftp.config.gui...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.ftp.control.gui...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.ftp.sampler...
  [javadoc] Loading source files for package org.apache.jmeter.protocol.jdbc...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.jdbc.config...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.jdbc.processor...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.jdbc.sampler...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.java.config...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.java.config.gui...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.java.control.gui...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.java.sampler...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.java.test...
  [javadoc] Loading source files for package org.apache.jorphan.collections...
  [javadoc] Loading source files for package org.apache.jorphan.exec...
  [javadoc] Loading source files for package org.apache.jorphan.gui...
  [javadoc] Loading source files for package org.apache.jorphan.gui.layout...
  [javadoc] Loading source files for package org.apache.jorphan.io...
  [javadoc] Loading source files for package org.apache.jorphan.logging...
  [javadoc] Loading source files for package org.apache.jorphan.math...
  [javadoc] Loading source files for package org.apache.jorphan.reflect...
  [javadoc] Loading source files for package org.apache.jorphan.test...
  [javadoc] Loading source files for package org.apache.jorphan.util...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.ldap.config.gui...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.ldap.control.gui...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.ldap.sampler...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.tcp.config.gui...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.tcp.control.gui...
  [javadoc] Loading source files for package 
org.apache.jmeter.protocol.tcp.sampler...
  [javadoc] Loading source files for package 
org.apache.jmeter.examples.sampler...
  [javadoc] Loading source files for package 
org.apache.jmeter.examples.sampler.gui...
  [javadoc] Loading source files for package 
org.apache.jmeter.examples.testbeans.example1...
  [javadoc] Loading source files for package 
org.apache.jmeter.examples.testbeans.example2...
  [javadoc] Loading source files for package 

buildbot failure in on jmeter-trunk

[buildbot]

The Buildbot has detected a new failure on builder jmeter-trunk while building 
. Full details are available at:
https://ci.apache.org/builders/jmeter-trunk/builds/3365

Buildbot URL: https://ci.apache.org/

Buildslave for this Build: bb_slave1_ubuntu

Build Reason: The AnyBranchScheduler scheduler named 'on-jmeter-commit' 
triggered this build
Build Source Stamp: [branch jmeter/trunk] 1818934
Blamelist: pmouawad

BUILD FAILED: failed shell_3

Sincerely,
 -The Buildbot

[GitHub] jmeter pull request #362: Fix sonar issue in PR #355

[ham1]

GitHub user ham1 opened a pull request:

https://github.com/apache/jmeter/pull/362

Fix sonar issue in PR #355



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/ham1/jmeter boundary_extractor_refactor

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/jmeter/pull/362.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #362


commit 4b75da00150395ff0fc31707d3f20967682e58aa
Author: Graham Russell 
Date:   2017-12-21T13:35:02Z

Fix sonar issue in PR #355




---

Re: add hooks to JMeter for websocket plugin

[Peter Doornbosch]

Thanks Philippe, great.
Looks good so far.
If i'd have any feedback i'll let you know.
Regards
Peter


2017-12-20 10:49 GMT+01:00 Philippe Mouawad :
> Hello Peter,
> Thanks for your ideas and PR which has been merged with slight
> modifications.
>
> Tests and feedback very welcome.
> Regards
> Philippe
>
> On Sun, Dec 17, 2017 at 8:48 PM, Peter Doornbosch <
> peter.doornbo...@gmail.com> wrote:
>
>> Hi,
>>
>> In my JMeter websocket plugin (
>> https://bitbucket.org/pjtr/jmeter-websocket-samplers) I want to hide
>> advanced options for first time users. One of my design goals for this
>> plugin was to make it very easy to use, so that first-time users are
>> not overwhelmed with loads of options they don't know how to use and this
>> will only confuse them. Up till now, i think i succeeded (but of course
>> that's for others to judge ;-)).
>>
>> However, i'm now at the point that i need to add functionality that will
>> only be used by advanced users. To keep the UI simple and clear for others,
>> these advanced options must be explicitly enabled. Currently, this is done
>> by setting a JMeter property. It works, but what i would like is that users
>> can simply enable this (and other) option(s) by adding a special Config
>> element in their test plan and enable/disable the options they want to use.
>> This keeps the settings (e.g. whether options are enabled or not) in the
>> testplan, which will avoid surprises when a testplan that relies on these
>> features is being loaded in a JMeter instance that doesn't have the
>> property set.
>>
>> I've been experimenting with this approach and i'm pretty happy with it.
>> However, to make it work correctly, i need some extra hooks in JMeter.
>>
>> The proposed changes explained below, are committed to my JMeter fork at
>> https://github.com/ptrd/jmeter/commits/more-hooks-for-
>> testplan-modifications
>> .
>>
>> Change 1
>> See
>> https://github.com/ptrd/jmeter/commit/a968db8bb63d16d6ea176ed00c0746
>> 68958b7544
>> A "removed" method is added to TestElement, so a test element can react on
>> it being removed from the test plan.
>> I need this for the functionality explained above, because when the user
>> removes the special Configuration element that enables certain options,
>> these options must be disabled.
>>
>> Change 2
>> See
>> https://github.com/ptrd/jmeter/commit/50e326ff824e231961308eed5185ae
>> af2d894f60
>> A "TestPlanListener" is added, and its "testPlanCleared" method is called
>> when the test plan is clear.
>> Similarly to change 1, when a testplan is closed and another one is loaded,
>> i need a hook to reset the state of the advanced options.
>> Of course, when included in JMeter, the TestPlanListener should probably be
>> extended with some other methods to make it a consistent interface; for now
>> i just wanted to create a working proof-of-concept.
>>
>> If you're interested in how these additions are used in the plugin, please
>> see this branch:
>> https://bitbucket.org/pjtr/jmeter-websocket-samplers/
>> commits/branch/configure-options-in-testplan
>> .
>>
>> Actually, i need one more feature, namely that a config element can be
>> declared as "singleton" so it can't be added multiple times, but i haven't
>> developed code for that yet - will follow later.
>>
>> I hope you are willing to add these hooks to JMeter, as, IMHO, these are
>> generic hooks that can be useful for others and other use cases as well.
>> For example, it has always frustrated me that the SSL keystore cannot be
>> configured in a testplan; which such additions as proposed here, i think
>> that would become possible too.
>> Of course, if you have any thoughts about how the proposed changes can be
>> improved, i'm happy to discuss it with you.
>>
>> Kind regards,
>> Peter Doornbosch
>>
>
>
>
> --
> Cordialement.
> Philippe Mouawad.

Re: [GitHub] jmeter issue #358: Checkstyle: LineLength max 165, AnonInnerLength 45 and ot...

[Peter Doornbosch]

Sorry, for the confusing email formatting. Of course, the way it is
displayed in the mail does not reflect the original source code
formatting, but i think you catch my point: i'm not very happy with
breaking up lines that are only 120 chars long..

Cheers
Peter


2017-12-21 10:37 GMT+01:00 Peter Doornbosch :
> I wonder why you break up lines like these
>
>  useBrowserCompatibleMultipartMode = new
> JCheckBox(JMeterUtils.getResString("use_multipart_mode_browser")); //
> $NON-NLS-1$
>
> into
>
> useBrowserCompatibleMultipartMode =
> new
> JCheckBox(JMeterUtils.getResString("use_multipart_mode_browser")); //
> $NON-NLS-1$
>
> IMHO this does not improve readability, but makes it worse.
>
> Moreover, the description says something about "max line length 165",
> the line above is only 120 chars.
>
> I admit max line length is also a matter of personal style, but i
> couldn't find any docs about what JMeter prescribes in this matter.
> Given no official rules, i would be careful with changes that won't be
> seen as improvements by all of us (your PR does contain a lot of
> changes that _are_ evident improvements).
>
> My 2cts.
>
> Peter
>
> 2017-12-21 8:33 GMT+01:00 ham1 :
>> Github user ham1 commented on the issue:
>>
>> https://github.com/apache/jmeter/pull/358
>>
>> Sorry, yeah I see how this is difficult to review. I tend to change 
>> things as I see them in the file I'm editing. I will revise and split it up 
>> into one type of change per commit, is that ok? It might be more than 10 
>> files but should be easier to review.
>>
>>
>> ---

Re: [GitHub] jmeter issue #358: Checkstyle: LineLength max 165, AnonInnerLength 45 and ot...

[Peter Doornbosch]

I wonder why you break up lines like these

 useBrowserCompatibleMultipartMode = new
JCheckBox(JMeterUtils.getResString("use_multipart_mode_browser")); //
$NON-NLS-1$

into

useBrowserCompatibleMultipartMode =
new
JCheckBox(JMeterUtils.getResString("use_multipart_mode_browser")); //
$NON-NLS-1$

IMHO this does not improve readability, but makes it worse.

Moreover, the description says something about "max line length 165",
the line above is only 120 chars.

I admit max line length is also a matter of personal style, but i
couldn't find any docs about what JMeter prescribes in this matter.
Given no official rules, i would be careful with changes that won't be
seen as improvements by all of us (your PR does contain a lot of
changes that _are_ evident improvements).

My 2cts.

Peter

2017-12-21 8:33 GMT+01:00 ham1 :
> Github user ham1 commented on the issue:
>
> https://github.com/apache/jmeter/pull/358
>
> Sorry, yeah I see how this is difficult to review. I tend to change 
> things as I see them in the file I'm editing. I will revise and split it up 
> into one type of change per commit, is that ok? It might be more than 10 
> files but should be easier to review.
>
>
> ---

[GitHub] jmeter issue #231: WIP: timer that produces poisson arrivals with given cons...

[vlsi]

Github user vlsi commented on the issue:

https://github.com/apache/jmeter/pull/231
  
>Could you confirm your component implements the following features:

That's true.


---