Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Request: WeightedSwitchController #93

Closed
MaksSieve opened this issue Apr 15, 2022 · 12 comments
Closed

Feature Request: WeightedSwitchController #93

MaksSieve opened this issue Apr 15, 2022 · 12 comments
Labels
enhancement New feature or request

Comments

@MaksSieve
Copy link
Contributor

MaksSieve commented Apr 15, 2022

I'd like to propose some massive but easy changes.

In my attempts to implement dsl for WeighedSwitchController(bzm plugin) I have stucked with inability to build TestElement from BaseTestElement because it's under protected method.

The main problem is in need to pass TestElement object to addTestElement method of any GenericController.

So I don't see any problem to make public and give more flexibity to create new components. But maybe there are another possibilities to make this.

@rabelenda
Copy link
Contributor

rabelenda commented Apr 15, 2022

What about modelling weighted switch controller like this:

weightedSwitchController()
  .child(25, mySampler)
  .child(75, myOtherSampler)
  .children(...) // for unweighted elements like pre, post, config, timers, assertions & listeners elements. If a sampler is added here you might take as default weight the same default the plugin uses (100).

and child receiving just an int and DslTestElement as parameters?

with such approach you wouldn't need the getName and getGuiClass public, avoiding those methods to interfere with user experiencing when searching for methods for further tune configuration, and wouldn't either need to create samplers before hand and call a potentially public addTestElement method?

@rabelenda
Copy link
Contributor

If you think that approach is good, and getName & getGuiClass are no longer needed as public, we can make a quick release to remove such methods and avoid users depending on something they actually may not need, simplifying also general users API.

@MaksSieve
Copy link
Contributor Author

MaksSieve commented Apr 15, 2022

the problem is that we want to pass dsl elements to our methods such .child(25, mySampler) in your example where mysampler is one of dsl wrappers for jmeter samplers.

but then we need to configure weighted controller with Jmeter TestElements which primarily are building with buildTestElement() method from corresponding DslTestElements which in its turn are protected and unavailable to use :(

@rabelenda
Copy link
Contributor

rabelenda commented Apr 15, 2022

Have you considered overwriting buildTreeUnder method? With that you can build all children test elements and then tune controller configuration according to final testElements in subtree. I know it might be more complex to implement than potential alternatives, but in the end would keep end users api clean and simple and in any case weighted controller internal logic and configuration is complex by itself (non standard JMeter way of creating and configuring controllers) so seems reasonable that dsl class needs some extra logic.

@kirillyu
Copy link
Contributor

Maybe use float or double as first arg of child?

@MaksSieve
Copy link
Contributor Author

For now buildTreeUnder is a sort of mystery for me) I don't fully understand what I should do with its result.
Main questions is how to divide the true child of controller from subchilds in the result of buildTreeResult.

@MaksSieve
Copy link
Contributor Author

Maybe use float or double as first arg of child?

of course) i think integers here are only for representation of numbers as such

@MaksSieve
Copy link
Contributor Author

MaksSieve commented Apr 15, 2022

@rabelenda May you add some docs about buildTreeUnder() methods? and may be provide some examples of using them.

@rabelenda
Copy link
Contributor

rabelenda commented Apr 15, 2022

Hello,

buildTreeUnder already has some java doc on it:

/**
   * Builds the JMeter HashTree for this TestElement under the provided tree node.
   *
   * @param parent the node which will be the parent for the created tree.
   * @param context context information which contains information shared by elements while building
   * the test plan tree (eg: adding additional items to test plan when a particular protocol element
   * is added).
   * @return The tree created under the parent node.
   * @since 0.17
   */
  HashTree buildTreeUnder(HashTree parent, BuildTreeContext context);

Maybe we could improve it as to make it more clear to devs how and when to override it.

In the mean time you can check the logic associated in BaseTestElement:

public HashTree buildTreeUnder(HashTree parent, BuildTreeContext context) {
  return parent.add(buildConfiguredTestElement());
}

Or any of the classes that implement the method (you can use IDE features to list and navigate to all clases implementing an interface method).

For the particular case of WeightedSwitchController, there is no similar logic to what you will need, since as I mentioned, this controller has some specific non standard way of configuring (by getting info from children).

In any case, you might get inspiration from RpsThreadGroup implementation:

public HashTree buildTreeUnder(HashTree parent, BuildTreeContext context) {
    HashTree ret = parent.add(buildConfiguredTestElement());
    HashTree timerParent = counting == EventType.ITERATIONS ? ret.add(buildTestAction()) : ret;
    timerParent.add(buildTimer());
    children.forEach(c -> context.buildChild(c, ret));
    return ret;
  }

And review, with a debugger, the resulting ret hashTree after adding children elements, and configuring the configuredTestElement with children and provided weights info.

If you find it complex, since as previously mentioned this particular scenario is, let us know and we may invest on it and try to implement something.

Regards

@rabelenda rabelenda added the enhancement New feature or request label Apr 15, 2022
@rabelenda
Copy link
Contributor

Hello, was the provided info helpful?

Regards

@MaksSieve
Copy link
Contributor Author

hi!
Yes! explanations helped a lot, now i'm testing my implementations inside company. I expect to make PR in the middle of this week.

@rabelenda
Copy link
Contributor

Awesome! looking forward for it! :)

@MaksSieve MaksSieve changed the title Feature Request: let buildTestElement be public. Feature Request: WeightedSwitchController Apr 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants