abstracta / jmeter-java-dsl

Simple JMeter performance tests API
https://abstracta.github.io/jmeter-java-dsl/
Apache License 2.0
477 stars 59 forks source link

Implemented Simple Controller #222

Closed ben-norton-anaplan closed 1 year ago

rabelenda commented 1 year ago

Hello, thank you very much for this contribution!

Can you clarify when do you use simple controllers over transaction controllers? It would be nice to provide users with clear indications on when makes sense to use one over the other.

Regards

FakeLogin commented 1 year ago

I'll indulge myself in answering instead of the author.

Simply put: Simple controller itself does not create any extra entities in the results list.

In the given set up

image

The output will be:

image

E.g. Transaction controller will create an extra entry in the results list. Either as a wrapper or directly, while Simple controller does not. While there are different approaches on how to structure scripts, Simple controller might be of use.

rabelenda commented 1 year ago

Thank you @FakeLogin for your contribution!

I knew about such difference, but has this been hardly a scenario, in my personal experience, to use simple controller instead of transaction controller. I wonder if it makes sense to have them both grouped under same element, since they both accomplish a similar use case (grouping requests and defining the scope of processors, assertions, listeners, etc). In the scenario of transaction controller there is also the additional feature of getting measurements (sampleResults) for the entire test plan section.

Maybe something like

transactionController()
  .withoutSampleResults()
  .children(...)

On one hand, this would avoid having to remember to use one over the other, and centralize their usage. On the other hand, it would require for JMeter users to remember that simple controllers are "hidden" in such element, and would also require adding some corner case logic to transaction controller (allow to not set a name, and to disable SampleResults).

I think in the end is preferable just having simpleController (as in the pull request) and in user guide describe both of them under existing group requests user guide section.

Additional question: do you see any usage for setting a name in simpleController? I would consider just having a builder without a name, since I don't recall any "functional" usage for the name in simple controllers.

Eg:

simpleController(
  httpSampler("http://myservice"),
  httpSampler("http://myservice/users"),
  responseAssertion().
    .containsSubstrings("OK")
)
FakeLogin commented 1 year ago

Regarding keeping option of setting a name - I'm fervently in favor of it. (Re)naming is something I use extensively in every project. When a scenario growth up to 100+ elements, it's only the naming that keeps it all together. Moreover, Simple Controller's primary function in my case is keeping elements logically organised, which implies not just naming but rather naming convention.

As for your example above - .withoutSampleResults() - it looks tempting. I can instantly imagine it to be generic enough to be applicable to any TestElement. On the other hand, I totally agree with adherence to SimpleController as a well-known approach.

And just to elaborate a little more on 'Naming' topic: I sometimes struggle not being able to set names to all the elements. Just an example: I used to rename, say, regex extractor with the name of a variable it extracts. It makes so much easier to sneak preview what's inside without looking inside. Same for Header managers and basically all the elements.

rabelenda commented 1 year ago

Thank you again @FakeLogin for your insights!

But, keep in mind that you are no longer tied to limitations/visualization of JMeter. With JMeter DSL you can define Java methods and even classes to organize (and even reuse) parts of the test plan.

Eg:

private DslSimpleController login() {
  return simpleController(
    httpSampler("https://myservice"),
    httpSampler("https://myservice/login")
      .post("{\"user\":\"test\",\"pass\":\"test\"}", ContentType.APPLICATION_JSON),
  );
}

private DslSimpleController addProduct() {
  return simpleController(
    httpSampler("https://myservice/products")
      .post("{\"name\":\"test\"}", ContentType.APPLICATION_JSON),
  );
}

@Test
public void test() {
  testPlan(
    threadGroup(1, 1,
      login(),
      addProduct()
    )
  ).run();
}

Above scenario uses the new element in this PR. And I think this proves also that having it, simplifies modularization since you can always return such element and easily add it in a test plan (previously it required some handling or arrays that was not as clean).

Even though JMeter DSL provides means to generate a JMX from the DSL test plan, or even visualize the test plan in JMeter GUI, those are just side/helper methods to ease transition or provide integration with other JMeter ecosystem tools.

That being said, and if the name of the controller does not affect in any way functionally the test plan, I am inclined to not provide the option to set a name, as a way to keep the API simpler and avoid users having to understand when does it make sense to use a name or not in a controller (the fewer options, the simpler).

Regarding the use of names in the API, for example in transaction controllers, we encourage (and even force) to set it in the API in scenarios where the name is relevant.

FakeLogin commented 1 year ago

Thank you for all the efforts on the DSL project, can't imagine how much it takes. And for this instant feedback.

Just to address all the aspects and corner cases on naming. There is a Switch Controller and Module Controller that depends on precise naming. For that matter Simple Controller will be out of use. Could be replaced with Transaction Controller or Test Fragment controller (the letter is not fully implemented yet) but both seems kind of a workaround.

Globally speaking, it boils down to how close should DSL be translated to original jmx script. And that's a great question to my mind. You are totally right pointing to advanced capabilities of keeping DSL project well structured and organised. I can't provide any other really strong arguments in advocacy of following JMeter conventions. Yet it could be the whole other vision if decoupling with JMeter script management.

rabelenda commented 1 year ago

Thank you very much @FakeLogin for your understanding and providing such good feedback for potential improvements.

Regarding Test Fragment & Module Controller, I doubt there is a real need for their support, but I am eager to hear for use cases and potential needs. We do support in jmx2dsl conversion of JMX files containing test fragments & module controllers, and we just convert them to java method definitions and their associated usage.

Regarding your final point, I see in general the initial need/desire for JMeter users to have a direct mapping to all options JMeter provide. But I think several are actually requirements due to JMeter GUI nature that are not necessary to move to the DSL java/code based nature, or are just JMeter backwards compatibility issues. Additionally, in several cases we change some names or change the way things are defined to avoid common JMeter usage pitfalls and to ease the learning curve for people with no JMeter experience. So, in general, many times is not easy to find the proper balance between the two main users profiles. We do try to keep it balanced, and hope that you (the community) help us in this regard.

rabelenda commented 1 year ago

@ben-norton-anaplan , can you please review if makes sense to remove the name from the builder for your use case? If so, we can just merge the PR as is, modify a little to accommodate to the proposed changes (user guide and builder parameters) and release it in the next version.

andy-tarr commented 1 year ago

@rabelenda whilst I agree with @FakeLogin that naming a simple controller does not have a 'functional' use, it does assist users finding elements in the test plan whilst writing tests using the DSL (via showInGui()) and also whilst debugging scripts. For this reason I think the pros outweigh the cons and naming the simple controller should be included.

rabelenda commented 1 year ago

Thank you @andy-tarr for your feedback.

I merged the PR and added an option to allow not setting the name, among other small changes for consistency and additional documentation/clarification.