You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by blindio <gi...@git.apache.org> on 2016/10/18 21:41:40 UTC
[GitHub] jmeter pull request #233: Bug 60274
GitHub user blindio opened a pull request:
https://github.com/apache/jmeter/pull/233
Bug 60274
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Nordstrom/jmeter 60274
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/jmeter/pull/233.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 #233
----
commit 732b0e23a7737962c71de23b9a76ac888191bd48
Author: Thorson Little <th...@nordstrom.com>
Date: 2016-06-01T01:49:16Z
initial import with working gui
commit c1e5d984687313f4225ba35fefd2b45d57108466
Author: Thorson Little <th...@gmail.com>
Date: 2016-06-02T19:10:02Z
Refactored for readability plus some bug fixes
commit 86f2dedc60325ddbd56071d990ffc4dbef7dd7ea
Author: Thorson Little <th...@gmail.com>
Date: 2016-06-02T20:20:13Z
Removed randomizer object since I determined that new WeightedDistCtrl
object were created for each thread thus new Random objects
commit 0e26a2ca5f8958fe9a86fd83ae7207d865a2af99
Author: Thorson Little <th...@gmail.com>
Date: 2016-06-02T21:45:27Z
Fixed issue when only zero-weight children exist
commit d56d436e75b1667ea4235517b7f823aceab4a245
Author: Thorson Little <th...@gmail.com>
Date: 2016-06-02T23:58:08Z
added unit tests, fixed a null pointer error bug revealed by unit
tests. the unit test testDistributionWithAllDisabled fails by throwing
all of the results to a single test elment when none should be called.
will investigate
commit 553b5d9b4402d893951344826b8ae7d6313af6c0
Author: Thorson Little <th...@gmail.com>
Date: 2016-06-03T23:19:10Z
Added unit tests for various sampler configurations. fixed bug where
disabled samplers are eliminated in addTestElement. added an
abstraction over the randomizer for unit-test-ability
commit 31214ba265ce3e0774ba4a7fd48e13e8a842a6e2
Author: Thorson Little <th...@gmail.com>
Date: 2016-06-05T08:35:39Z
removed referenced to perThreadCheckbox
commit 7807159a5ab0e7ed69ad1390acc7f469f51afcad
Author: Thorson Little <th...@gmail.com>
Date: 2016-06-07T21:50:01Z
Added functionality to use variables for weights and will render those
variables in a column
commit b544c10a9312e9711e8962a78dd0c62ac591f012
Author: Thorson Little <th...@gmail.com>
Date: 2016-06-08T18:38:58Z
Added xdocs component reference
commit 03c9d630d2e27878d87f6d6ceaf94500d7372a83
Author: Thorson Little <th...@gmail.com>
Date: 2016-06-08T23:32:04Z
Code cleanup complete, more unit tests to add, otehrwise I think that
this is done
commit eec359b7c33466891275568c134800dcb47fe902
Author: Thorson Little <th...@gmail.com>
Date: 2016-06-08T23:36:43Z
copy & paste error in saveservice.props created duplicate
InterleaveControl entry
commit 657419583651d2c6036b82ef5cb90bcb9b7ea6ba
Author: Thorson Little <th...@gmail.com>
Date: 2016-06-17T22:20:03Z
Code cleanup and more unit tests
commit 6a306f8f814d64d06257690be1c42a2d2061ed1d
Author: Thorson Little <th...@gmail.com>
Date: 2016-06-23T23:04:01Z
Better comments and more unit tests
commit 6fbb363c79654072511e6dcc4058c6db1e6da809
Author: Thorson Little <th...@gmail.com>
Date: 2016-06-23T23:10:26Z
Added one more unit test
commit 5b5670e53ca858f9a5daab23fba9b1e53831f321
Author: Thorson Little <th...@gmail.com>
Date: 2016-06-24T21:58:59Z
variable and method renaming
commit eebb29a6407cd1ea658dd64b11fe97e713d97e9d
Author: Thorson Little <th...@gmail.com>
Date: 2016-06-28T17:10:53Z
renamed seed to generator seed
commit 0938aed61bc65d28dbd416a6348008cc16bec060
Author: Thorson Little <th...@gmail.com>
Date: 2016-06-28T17:53:09Z
Merge branch 'trunk' of https://github.com/Nordstrom/jmeter into trunk
commit f55d0e2005d0bcebf39725b2db9be05b0902df6b
Author: Thorson Little <th...@gmail.com>
Date: 2016-06-28T18:03:50Z
Revert "Merge branch 'trunk' of https://github.com/Nordstrom/jmeter into trunk"
This reverts commit 0938aed61bc65d28dbd416a6348008cc16bec060, reversing
changes made to eebb29a6407cd1ea658dd64b11fe97e713d97e9d.
commit 6e4179ac513aa97385eb7b8a8bdba71162a378b3
Author: Thorson Little <th...@gmail.com>
Date: 2016-06-28T21:57:06Z
manually merged due to confusion with earlier automated merge
commit e3199a46a5bc5912f90ffe9d1394ae20c50c3434
Author: Thorson Little <th...@gmail.com>
Date: 2016-06-28T22:31:58Z
fixing logging messages
commit b4fe985a5560dd1166a40e8395be1cb27d1fb4d9
Author: Thorson Little <th...@gmail.com>
Date: 2016-10-05T23:33:10Z
Merge branch 'trunk' of https://github.com/apache/jmeter into trunk
commit 84908bea2279749e922160ca512ebf0d85f05527
Author: Thorson Little <th...@gmail.com>
Date: 2016-10-07T19:55:09Z
Refactored WeightedDistributionControllerTableModel to break out internal
classes so they coulb be made public and would not break JMeterTest when
it instantiates Serializable classes.
Added new lines to some files to meet style guidelines
Moved gui classes to sub package so the classes from
WeightedDistributionControllerTableModel would not pollute gui package
commit e8ef18c72ef5b5659f8d07dd1a8d055b56878a45
Author: Thorson Little <th...@gmail.com>
Date: 2016-10-07T22:17:36Z
Added french translations to resource files
Updated FILEVERSION checksum in SaveService
commit 78bab07ccdbade1ef971d252cf80f9035e4bbad7
Author: Thorson Little <th...@gmail.com>
Date: 2016-10-07T23:11:11Z
Merge branch 'trunk' of https://github.com/apache/jmeter into trunk
Updated SaveService.FILEVERSION with new checksum
commit 104dd2575cb61dafa5b39d10e4284f3b2eac4a20
Author: Thorson Little <th...@gmail.com>
Date: 2016-10-11T18:07:57Z
Merge branch 'trunk' of https://github.com/apache/jmeter into trunk
commit c2b090f66098cffca99c4f19fbf1d3d7bd9b0119
Author: Thorson Little <th...@gmail.com>
Date: 2016-10-18T20:04:50Z
Merge branch 'trunk' of git://git.apache.org/jmeter into trunk
commit aab9c83a0e9889d17a93bd8f3808c97d1d2c04f7
Author: Thorson Little <th...@gmail.com>
Date: 2016-10-18T20:53:05Z
Added apache license header to two files
commit 25db3b56a21b32f6a30a5c0f1c497ee8a0f834e3
Author: Thorson Little <th...@gmail.com>
Date: 2016-10-18T21:28:19Z
Fix to 60274
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #233: Bug 60274
Posted by vlsi <gi...@git.apache.org>.
Github user vlsi commented on the issue:
https://github.com/apache/jmeter/pull/233
> I will test and consider a solution
The question is if that is feasible at all to support run-time variable changes and keep the performance sane
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #233: Bug 60274
Posted by blindio <gi...@git.apache.org>.
Github user blindio commented on the issue:
https://github.com/apache/jmeter/pull/233
All build and test errors that I believe that I introduced are resolved. Please tell me if this is incorrect.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #233: Bug 60274
Posted by blindio <gi...@git.apache.org>.
Github user blindio commented on the issue:
https://github.com/apache/jmeter/pull/233
The reason that I do not rely on the seed is because, as I understand it,
you can count on seed to be consistent within am implementation of random,
but not across implementations.
Users of 3rd party JVMs may have consistent failures in the unit tests
because their implementation of Random provides a different distribution
than those using the Open/Oracle JVM.
Since the intention of the test is to determine if all potential random
numbers bin correctly, the systematic approach guarantees correct binning.
Furthermore, magic numbers in unit tests should be avoided if at all
possible.
-Thorson
On Nov 12, 2016 11:33 PM, "Vladimir Sitnikov" <no...@github.com>
wrote:
> *@vlsi* commented on this pull request.
> ------------------------------
>
> In src/components/org/apache/jmeter/control/WeightedDistributionController
> .java
> <https://github.com/apache/jmeter/pull/233#pullrequestreview-8310668>:
>
> > + } else if (currSubCtrl instanceof TestElement) {
> + nextSubController = (TestElement) currSubCtrl;
> + }
> +
> + if (!(nextSubController.isEnabled()) && (nextSubController instanceof Controller || nextSubController instanceof Sampler)) {
> + setNextSubController();
> + }
> + } else {
> + nextSubController = null;
> + }
> + }
> +}
> +
> +/**
> + * This abstraction is necessary for unit tests, in which case a deterministic
> + * generator replaces the random one
>
> Why can't seed be used in order to make test consistent?
> There are at least two ways:
> 1) Hard-code seed value right into the test
> 2) Use random seed, and enable configuring the seed from the command line.
> That way if a test with some specific seed value fails, we can reproduce it
> by supplying that particular value.
>
> \u2014
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/jmeter/pull/233#pullrequestreview-8310668>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AGA4ZdY5iHAy5B2rz84YFWBlGCkstdOlks5q9r1LgaJpZM4KaUrV>
> .
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #233: Bug 60274
Posted by blindio <gi...@git.apache.org>.
Github user blindio commented on the issue:
https://github.com/apache/jmeter/pull/233
Not yet, things got hectic for me at work, will resolve in a little while
On Nov 12, 2016 5:38 AM, "Philippe M" <no...@github.com> wrote:
> Hi @blindio <https://github.com/blindio> ,
> Did you have time working on the reported issues ?
> Thanks
>
> \u2014
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/jmeter/pull/233#issuecomment-260122682>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AGA4Zcl0l-_4oWHUG8EsubQ6Tkdg-xFmks5q9cFigaJpZM4KaUrV>
> .
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #233: Bug 60274
Posted by blindio <gi...@git.apache.org>.
Github user blindio commented on the issue:
https://github.com/apache/jmeter/pull/233
I look forward to your feedback
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #233: Bug 60274
Posted by blindio <gi...@git.apache.org>.
Github user blindio commented on the issue:
https://github.com/apache/jmeter/pull/233
Thank you Philippe, I will make these changes in the next day or so
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter pull request #233: Bug 60274
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/jmeter/pull/233
---
[GitHub] jmeter issue #233: Bug 60274
Posted by pmouawad <gi...@git.apache.org>.
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/233
Hi @blindio ,
Did you have time working on the reported issues ?
Thanks
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #233: Bug 60274
Posted by pmouawad <gi...@git.apache.org>.
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/233
Hi Thorson,
Sorry for late reply, we are in the middle of 3.1 release preparation and I was on holiday.
Thanks again for you PR.
I started reviewing, few notes:
- WeightedDistributionController#initValueReplacer() => GuiPackage.getInstance() is null in Non GUI mode which is the expected mode for running load test. So if this is really needed (TestElement should avoid using GuiPackage.getInstance()) , the test should be done on nullity of GuiPackage.getInstance() to avoid Exception throwing and cached
- Same note for WeightedDistributionController#getNode()
RandomIntegerGenerator:
- When seed is not set, using ThreadLocalRandom.current() might be better
SubControllerIterator:
- Vector must be avoided
determineCurrentTestElement:
- Why do you test currSubController.isEnabled() ? Could you improve javadoc to make the algo clear for me ?
-Regarding code style, I'd prefer to have each class in its own file or static internal rather than having multiple classes in the same file (WeightedDistributionController => RandomIntegerGenerator, IntegerGenerator), SubControllerIterator might be better internal static ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #233: Bug 60274
Posted by blindio <gi...@git.apache.org>.
Github user blindio commented on the issue:
https://github.com/apache/jmeter/pull/233
Regarding the run time variable changes, that is a really good point. I
will test and consider a solution.
Thank you,
Thorson
On Nov 12, 2016 11:39 PM, "Vladimir Sitnikov" <no...@github.com>
wrote:
> *@vlsi* commented on this pull request.
> ------------------------------
>
> In src/components/org/apache/jmeter/control/WeightedDistributionController
> .java
> <https://github.com/apache/jmeter/pull/233#pullrequestreview-8310729>:
>
> > + * the index of the child node
> + * @return the child test element
> + */
> + public TestElement getChildTestElement(int idx) {
> + JMeterTreeNode childNode = getChildNode(idx);
> + return childNode == null ? null : childNode.getTestElement();
> + }
> +
> + /**
> + * Gets the cumulative probability.
> + *
> + * @return the cumulative probability
> + */
> + public int getCumulativeProbability() {
> + // recalculate if reset flag is set
> + if (resetCumulativeProbability) {
>
> Can you clarify when weight is recalculated?
> The documentation suggests that weight is recalculated at run-time,
> however the flag resetCumulativeProbability is reset at design time only.
> That is it looks like weight distribution would not account for the
> variable changes in the runtime. Is that the case? Can you make
> documentation/intention more clear?
>
> \u2014
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/jmeter/pull/233#pullrequestreview-8310729>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AGA4ZUhJMiF0gHbHIo8cWemAdoNZEbg-ks5q9r6wgaJpZM4KaUrV>
> .
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #233: Bug 60274
Posted by blindio <gi...@git.apache.org>.
Github user blindio commented on the issue:
https://github.com/apache/jmeter/pull/233
Hi Philippe-
I have not heard anything for a while and wanted to know if you had an idea when someone would be able to review this.
Thanks,
Thorson
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #233: Bug 60274
Posted by blindio <gi...@git.apache.org>.
Github user blindio commented on the issue:
https://github.com/apache/jmeter/pull/233
That is strange about the nesting not working, given that I manually tested
nesting, and it is one of the unit tests as well. I will investigate.
On Nov 13, 2016 12:08 AM, "Vladimir Sitnikov" <no...@github.com>
wrote:
> While being able to enable-disable and rename the children from WDC UI is
> nice, I wonder how many worms are hidden in that "keep WDC and children
> tree" consistent can.
>
> Nested "Weighted Distribution Controller" elements does not seem to work.
>
> [image: 2016-11-13 11 05 00]
> <https://cloud.githubusercontent.com/assets/213894/20244397/18f73646-a991-11e6-81bd-945e78f0823a.png>
>
> [image: 2016-11-13 11 05 04]
> <https://cloud.githubusercontent.com/assets/213894/20244398/21b7d0f6-a991-11e6-81cc-9f35087a9ce9.png>
>
> [image: 2016-11-13 11 05 09]
> <https://cloud.githubusercontent.com/assets/213894/20244400/25cf7d60-a991-11e6-9a44-b737539af738.png>
>
> \u2014
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/jmeter/pull/233#issuecomment-260172511>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AGA4ZYV83TGbaj8UNlbHMh0HQY71KA_Vks5q9sVvgaJpZM4KaUrV>
> .
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #233: Bug 60274
Posted by pmouawad <gi...@git.apache.org>.
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/233
It appears similar feature is now available a 3rd party plugin :
* https://jmeter-plugins.org/?search=Weighted
So I propose to close this one.
---
[GitHub] jmeter issue #233: Bug 60274
Posted by pmouawad <gi...@git.apache.org>.
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/233
Hi Again,
For info, in your commit, no need to include the HTML files, only XML for documentation.
Thanks
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #233: Bug 60274
Posted by pmouawad <gi...@git.apache.org>.
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/233
Thanks for this contribution !
We will review it ASAP and give you feedback.
Regards
Philippe M.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #233: Bug 60274
Posted by vlsi <gi...@git.apache.org>.
Github user vlsi commented on the issue:
https://github.com/apache/jmeter/pull/233
While being able to enable-disable and rename the children from WDC UI is nice, I wonder how many worms are hidden in that "keep WDC and children tree" consistent can.
Nested "Weighted Distribution Controller" elements does not seem to work.
<img width="691" alt="2016-11-13 11 05 00" src="https://cloud.githubusercontent.com/assets/213894/20244397/18f73646-a991-11e6-81bd-945e78f0823a.png">
<img width="694" alt="2016-11-13 11 05 04" src="https://cloud.githubusercontent.com/assets/213894/20244398/21b7d0f6-a991-11e6-81cc-9f35087a9ce9.png">
<img width="692" alt="2016-11-13 11 05 09" src="https://cloud.githubusercontent.com/assets/213894/20244400/25cf7d60-a991-11e6-9a44-b737539af738.png">
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #233: Bug 60274
Posted by vlsi <gi...@git.apache.org>.
Github user vlsi commented on the issue:
https://github.com/apache/jmeter/pull/233
Just in case, C and C1 has the following groovy post-processor: `vars.put("x", String.valueOf(1+Integer.parseInt(vars.get("x"))))`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jmeter issue #233: Bug 60274
Posted by blindio <gi...@git.apache.org>.
Github user blindio commented on the issue:
https://github.com/apache/jmeter/pull/233
Agreed, I need to find some time to consider this issue. I may be that the
variable resolution is not feasible without major changes.
Again, thank you,
Thorson
On Nov 13, 2016 1:18 AM, "Vladimir Sitnikov" <no...@github.com>
wrote:
> I will test and consider a solution
>
> The question is if that is feasible at all to support run-time variable
> changes and keep the performance sane
>
> \u2014
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/jmeter/pull/233#issuecomment-260174948>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AGA4ZfYRN5Lj1vAmy9H9q-Xv_JaApE0lks5q9tXLgaJpZM4KaUrV>
> .
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---