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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
