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

GROUP syntax for grouping keywords and control structures #5257

Closed
wenkai5566 opened this issue Nov 7, 2024 · 26 comments
Closed

GROUP syntax for grouping keywords and control structures #5257

wenkai5566 opened this issue Nov 7, 2024 · 26 comments
Assignees
Milestone

Comments

@wenkai5566
Copy link

The proposed BLOCK keyword would allow users to logically group related steps, improving test case organization, readability, and making the logs clearer and more structured by avoiding clutter at the top level.

Currently, to achieve logical grouping, users often create custom keywords to wrap related steps, leading to extra development and maintenance efforts. With BLOCK, users can group steps directly within test cases, minimizing the need for custom keywords.

Example:

*** Test Cases ***
Test Server Configuration
    BLOCK  Check Network Configuration
        Check IP Configuration    ${dut1}    ${dut1_ip}
        Check Subnet Mask    ${dut1}    ${dut1_subnet}
        Check Gateway    ${dut1}    ${dut1_gateway}
    END
    BLOCK  Check Service Status
        Check Service Status    ${dut1}    HTTP    status=active
        Check Service Status    ${dut1}    SSH      status=inactive
    END
@wenkai5566 wenkai5566 changed the title Feature Request: Add BLOCK Keyword to Group Related Steps Feature Request: Add BLOCK keyword to group related steps Nov 7, 2024
@pekkaklarck
Copy link
Member

I don't particularly like the proposed BLOCK syntax when used like this in the data. Creating separate keywords and using them to create the test would make it a lot easier to understand. I see some benefits in avoiding the need to create those keywords if they are used only once, but this usage alone wouldn't be worth adding this syntax. There are, however, two other usages where a block syntax would help:

  1. It would make it easy to create nested structures programmatically. We already have Creating user keywords with programmatic API #5119 about creating user keywords programmatically, but I'd actually prefer creating blocks instead. The usage would be something like this:

    block = test.body.create_block(name='Example name. This could be optional.')
    block.body.create_keyword('Log', ['Hello!'])
    block.body.create_if().body.create_branch(condition='$expr').body.create_keyword('Log', ['Hello in IF!'])
  2. This syntax could allow creating setups/teardowns with multiple keywords without Run Keywords. It would also support control structures. The problem is that at the moment setup/teardown must be a keyword, but it would be good to fix that limitation also otherwise. Another problem is that we needed to come up with a syntax for multiline setup/teardown. It would be nice if something like this worked:

    [Setup]    BLOCK
        Log    Hello!
        IF    $expr    Log    Hello in IF!
    END
    

    This part should get a separate issue, though, and could be implemented after the BLOCK syntax is implemented otherwise.

@pekkaklarck
Copy link
Member

I'll add this to the very initial RF 8.0 scope because #5119 is there already. I don't see any backwards compatibility problems with the change, so it could also be implemented earlier. I probably won't have time for it myself, but I'd be happy to help and review PRs.

@pekkaklarck pekkaklarck added this to the v8.0 milestone Nov 13, 2024
@pekkaklarck pekkaklarck changed the title Feature Request: Add BLOCK keyword to group related steps BLOCK syntax for grouping related steps Nov 13, 2024
@Tattoo
Copy link
Member

Tattoo commented Nov 13, 2024

Pinging in @asimell as this will have downstream effects on the Jenkins plugin

@emanlove
Copy link
Member

emanlove commented Nov 14, 2024

I'm sorry but I really don't see how adding this BLOCK .. END syntax makes the overall test case more organized nor any more readable. How does this written syntax translate into the dynamic viewing of the log? Is it just more lines in the log? I see it as dramatically decreasing the readability. And if it adds no functionality, my first question, then what is the purpose?

@asimell
Copy link
Contributor

asimell commented Nov 14, 2024

This will probably have an impact on the Jenkins plugin even though individual keywords are only parsed for logging purposes and their data is not actually stored for the plugin to use.

@emanlove I don't necessarily see it increasing readability in the test case in itself. From Pekka's examples a simple #Check Network Configuration and #Check Service Status would be more readable in the test file IMO. However, I do see it increasing readability in the log file as technical keyword calls could be grouped logically. Especially in lengthier keywords and in cases a separate keyword would be used only once.

@pekkaklarck
Copy link
Member

As I commented earlier, I don't see this too important in normal data. Basically the proposed syntax would allow writing

*** Test Cases ***
Example
    BLOCK    Do something
        Action 1
        Action 2
    END
    BLOCK    Verify something succeeded
        Verify 1
        Verify 2
    END

instead of needing to create user keywords like

*** Test Cases ***
Example
    Do something
    Verify something succeeded

*** Keywords ***
Do something
    Action 1
    Action 2

Verify something succeeded
    Verify 1
    Verify 2

In the log file the result would be the same except in the former case the test would contain BLOCKs with child keywords and in the latter case the test would contain keywords with child keywords.

In most cases the latter approach would be better, and it would be obviously better if the created keywords could be reused. I can understand that someone is worried that this makes it easy to create bad tests, but that's possible already today by not using abstractions at all:

*** Test Cases ***
Example
    # Do something
    Action 1
    Action 2
    # Verify something succeeded
    Verify 1
    Verify 2

It can be hard to motivate some users to change the above example to use proper user keywords because that requires some work. Changing it to use BLOCKs would be simpler and, as already noted, there's practically no difference when you look at the log file. That said, it would probably be good to have a linter rule for prohibiting BLOCKS at least on the test level,

All in all, I see some benefits with the BLOCK syntax when used like above. These benefits probably wouldn't be worth the implementation effort and added complexity Robot itself and to the whole ecosystem. It is a good point that tools processing output.xml would need to take <block> elements into account.

The main reason I believe this would be a good idea is that this syntax would provide a convenient programmatic API for creating structured tests and keywords. This has been requested multiple times especially in the RPA context. The most recent example #5119 that proposed making it possible to add user keywords to tests directly. The BLOCK structure would make it possible to implement exactly this behavior without making existing robot.running.Keyword a really complicated hybrid object.

I pretty strongly believe making it possible to create structured tests/keywords programmatically is a useful feature. Support for simple named blocks is the simplest possible way I can think it could be implemented. We obviously could decide not so support BLOCK structures in data, but I think that would be really odd and wouldn't save much effort.

Another possible benefit is that BLOCKs could possibly work as setup/teardown. Because currently setup/teardown must always be a keyword, that requires more work, but having the syntax available would make the effort smaller.

@mikeleppane
Copy link

When I first encountered this proposal, I was somewhat skeptical. However, upon further reflection, I see several potential benefits. For one, tests lacking abstractions (or not utilizing the BDD approach) can be pretty difficult to read. The use of BLOCK introduces a simple way to add structure to your test code. Whether or not it should be applied at the test level is a matter of debate. As mentioned earlier, I find that BLOCK is most beneficial when used in conjunction with setup and teardown, as it allows for easily declaring multiline statements.

Additionally, this approach may also be suitable for a programmatic API. I don’t have a strong opinion on this matter since I rarely need to use the robot's API.

@Snooz82 Snooz82 self-assigned this Nov 19, 2024
@Snooz82 Snooz82 modified the milestones: v8.0, v7.2 Nov 20, 2024
@pekkaklarck
Copy link
Member

We decided to implement this and do it already in RF 7.2. The most important reasons are:

  • This provides a convenient API to create structured tests/tasks/keywords using pre-run modifiers, listeners, libraries, etc.
  • This also eases creating structured data. User keywords are typically preferred (especially if they can be reused), but in special cases creating structured blocks directly has benefits. Examples included data generation and cleaning up existing tests without no structure. In both of these usages it is convenient that variables are usable everywhere and there's no need to pass values to keywords as arguments and return them as return values.

@Snooz82, @aaltat and I will work on this tomorrow.

@pekkaklarck
Copy link
Member

When discussing about this, we weren't sure what to call these things. The proposed BLOCK is ok, but it can be confused with "blocking". Other alternatives include at least the following:

  • GROUP
  • STEP
  • KEYWORD
  • SEQUENCE
  • REGION

GROUP was our favorite, but we are open to other ideas still.

@test-fullautomation
Copy link

Why not just curly brackets { ... }. Not much to write, but very clear what it means.

@pekkaklarck
Copy link
Member

Using curly braces in this context would be pretty inconsistent. It would be a natural choice if we'd use them with other control structures such as IF, but now that we've settled to syntax like MARKER ... END, we should stick to it.

@test-fullautomation
Copy link

I agree to your concern about inconsistency.
I hear developers always complaining when they have to write much, that's the reason for the idea.

@mikeleppane
Copy link

We decided to implement this and do it already in RF 7.2. The most important reasons are:

  • This provides a convenient API to create structured tests/tasks/keywords using pre-run modifiers, listeners, libraries, etc.
  • This also eases creating structured data. User keywords are typically preferred (especially if they can be reused), but in special cases creating structured blocks directly has benefits. Examples included data generation and cleaning up existing tests without no structure. In both of these usages it is convenient that variables are usable everywhere and there's no need to pass values to keywords as arguments and return them as return values.

@Snooz82, @aaltat and I will work on this tomorrow.

Are you planning to support BLOCK/GROUP in the setup/teardown context as well? This aspect is the most interesting in terms of implementation.

@Snooz82
Copy link
Member

Snooz82 commented Nov 21, 2024

2. This syntax could allow creating setups/teardowns with multiple keywords without Run Keywords. It would also support control structures. The problem is that at the moment setup/teardown must be a keyword, but it would be good to fix that limitation also otherwise. Another problem is that we needed to come up with a syntax for multiline setup/teardown. It would be nice if something like this worked:

[Setup]    BLOCK
    Log    Hello!
    IF    $expr    Log    Hello in IF!
END

This part should get a separate issue, though, and could be implemented after the BLOCK syntax is implemented otherwise.

I think also this should be a separate issue and should then introduce IF, FOR, WHILE and GROUP to be possible in setups and teardowns.

Maybe even with line continuation?!

[Setup]    GROUP
...        Log    Hello!
...        IF    $expr    Log    Hello in IF!
...    END

@pekkaklarck
Copy link
Member

Yeah, we should allow any construct to be used as setup/teardown except for things like BREAK and RETURN. That requires a separate issue and part of that issue is deciding the syntax.

@mikeleppane
Copy link

I am still contemplating whether "GROUP" is the most suitable name for this. It appears somewhat unusual when read alone and even more so when used frequently. For example, Python employs "ExceptionGroup," which is both clear and unambiguous.

Suggestion: Could we consider "SECTION" or "CONTEXT" as alternatives?

@emanlove
Copy link
Member

emanlove commented Nov 21, 2024

I'm really confused by why one must have this test level Block ... End syntax to have a programmatic API for creating structured tests and keywords? Can't the api just have methods for creating keywords? I see that whatever implementation needed for such a Block ... End syntax would be the same/similar for having a programmatic API. Thus maybe an argument of why not create both. But if everyone agrees that the majority of what the BLOCK ... END syntax can and really should be a keyword - we are a keyword framework after all - and that it does not increase readability - some of us are saying it greatly decrease it - then why add this keyword syntax at all???

@Snooz82
Copy link
Member

Snooz82 commented Nov 21, 2024

@mikeleppane SECTIONS are in Robot Framework these *** Settings *** and *** Keywords *** and their content.

@emanlove I generally agree, with your idea. But i realised that this can really be helpfull.
One example where it might just not justify the effort and also does not help readability to create a keyword:

*** Test Cases ***
Test Customer Management
    Login To Customer Management    admin    123
    ${Customer_Name}     Get Customer Name     32
    @{Invoices}     Get Customer Invoices    32
    &{addr}    Get Customer Address    32
    ...
    ...  # here some more keywords that uses these data.
    ...

Grouping the fetching of customer data would make sense, to help readability.
As well in the log.html because it just looks like a keyword, as in the robot files, because it can be collapsed but you can still directly read the content.
Creating keywords in a Suite as local suite file keywords, may help structure long and complex test cases. However, it also obfuscates the actual content of the test, because you might have to jump from test to keyword to understand, to test to see the next, to keyword to understand, etc.

In this particular case, a keyword would be komplext, because it would have to return three values.
The variables here are all on LOCAL scope for the test. Creating keywords would create the need of handing over arguments or returning stuff.

*** Test Cases ***
Test Customer Management
    Login To Customer Management    admin    123
    GROUP    Fetch Customer Data
        ${Customer_Name}     Get Customer Name     32
        @{Invoices}     Get Customer Invoices    32
        &{addr}    Get Customer Address    32
    END
    ...
    ...  # here some more keywords that uses these data.
    ...

One other use-case and that is the one why i volunteered to implement it:
We are generating CODE with our tool. Not a running model.
And we want to be able to generate groups of keywords, that belong to a higher level funktion.
With keywords it would not be possible because the system where we are generating it from, works differently regarding arguments and return values. Therefore keywords are not possible.
These GROUPs are a perfect solution for this.

And last ,but not least:
This is a preparation to make Run Keywords obsolete.

@pekkaklarck
Copy link
Member

pekkaklarck commented Nov 21, 2024

@emanlove, #5119 proposal an API to create keywords programmatically. The problem was that robot.running.Keyword represents a keyword call and in practice only contains the name of the keyword to call and its arguments. We only resolve the keyword at runtime and the keyword implementation can then be either a library keyword or a user keyword with a body. You cannot thus directly create a test with a nested keyword structure, you only can create a keyword call and separately a user keyword matching it.

#5119 proposed making it possible to add child keywords directly to robot.running.Keyword. It would be technically possible, but it would be really confusing and it would also complicate execution logic. In normal cases we'd resolve the keyword the same way as now, but if the keyword has children, we wouldn't resolve it but instead start executing chlid keywords. There would be open questions like what to do with arguments if the keyword isn't actually resolved and run.

The block/group construct is a lot simpler. It's simply a container with a name used only for documentation purposes. Executing it means executiing its children.

@Snooz82
Copy link
Member

Snooz82 commented Nov 21, 2024

I just realized that this will also be handy for REPL (DebugLibrary & RobotDebug).

It is easier to understand as user, that you can create a Multi-Keyword action with the GROUP.
Because when starting with GROUP the REPL or debug console does know, that ENTER is just a new line and not an action execution.

@Snooz82 Snooz82 changed the title BLOCK syntax for grouping related steps GROUP syntax for grouping statements in Test|Task or Keywords. Nov 22, 2024
@pekkaklarck
Copy link
Member

One thing to decide is how to handle GROUP with templates.

We first need to decide do we want to support them together at all. I believe we should, because GROUP is only used for grouping and we support templates also with FOR and IF.

Assuming we want to support GROUP with templates, we need to decide how they should work if an iteration is skipped. Semantics related to that have just changed with normal templates, and nowadays a skipped iteration doesn't stop the whole test (#4426). Instead the test status is checked after all iterations have been run and is:

  • FAIL if any iteration failed.
  • PASS if there were no failures and at least one iteration succeeded.
  • SKIP if all iterations were skipped.

The following example illustrates how GROUP should work, in my opinion, under these semantics. Both groups should be executed with all of them.

*** Settings ***
Test Template      Run Keyword

*** Test Cases ***
PASS + SKIP + FAIL -> FAIL
    GROUP
        Log    Pass 1
        Skip    Skip 1
        Fail    Fail 1
    END
    GROUP
        Log    Pass 2
        Skip    Skip 2
        Fail    Fail 2
    END

PASS + SKIP -> PASS
    GROUP
        Log    Pass 1
        Skip    Skip 1
    END
    GROUP
        Log    Pass 2
        Skip    Skip 2
    END

SKIP + SKIP -> SKIP
    GROUP
        Skip    Skip 1.1
        Skip    Skip 1.2
    END
    GROUP
        Skip    Skip 2.1
        Skip    Skip 2.2
    END

I believe most of this will work as above with the basic implementation. The only possible problems I see are with error reporting. I just noticed templated IF and FOR have that kind of problems as well and thus reopened the aforementioned #4426. I plan to fix those problems and that probably also affects also GROUP, but there's also a risk for a conflict. Let's discuss on Slack how we should proceed @Snooz82. I can also take care of the template support with GROUP if you want.

@bhirsz
Copy link
Contributor

bhirsz commented Nov 26, 2024

What about BDDs? It was mentioned in the thread, but I see possible benefit if the GROUP could be aliased. Disclaimer: I don't use the BDDs as usually it looks too verbose to me. But with GROUP syntax it could change.

GROUP -> alias with Given, When, Then:

*** Test Cases ***
Old style BDD test case
    Given Customer Data Is Prepared
    And Input Table Is Empty
    When New Customer Is Registered
    Then Customer Is In Database
    And Customer Is Visible In Customers View

New style with aliased grouping
    GIVEN
        Customer Data Is Prepared
        Input Table Is Empty
    END
    WHEN  New Customer Is Registered  # inline? if it makes sense
    THEN
        Customer Is In Database
        Customer Is Visible In Customers View
    END

It could of course be written with GROUP and proper comments, like Given/When.. or Arrange/Act/Assert:

*** Test Cases ***
Test
    GROUP    Given
        Customer Data Is Prepared
        Input Table Is Empty
    END
    GROUP    When
        New Customer Is Registered  # inline? if it makes sense
    END
    GROUP   Then
        Customer Is In Database
        Customer Is Visible In Customers View
    END

Basically if the new syntax is introduced, I'm debating about recommended approach for users who use BDD style and if it makes sense to accomodate for them.

@pekkaklarck
Copy link
Member

pekkaklarck commented Nov 26, 2024

In BDD you want to have as easy to understand tests/examples as possible. Although the GIVEN/END block looks kind of nice, I don't see too much benefits with that compared to the normal Given/And style (where I typically like to use and instead of And to make continuation of the Given phase more explicit). Using Given etc. as group names will be supported out-of-the-box, but it doesn't look as nice as GIVEN/END. Anyway, I believe this is something we can return to later if there are needs.

@mikeleppane
Copy link

mikeleppane commented Nov 26, 2024

Yep. As a developer who primarily uses the BDD (Behavior Driven Development) style, I find little value in using GIVEN/END regarding readability. While I agree it looks nice, I wonder if the benefits justify the implementation effort.

@pekkaklarck pekkaklarck self-assigned this Dec 10, 2024
pekkaklarck added a commit that referenced this issue Dec 13, 2024
Functionality done and tested. Using with templates as well as documentation still missing.

---------

Co-authored-by: Pekka Klärck <[email protected]>
pekkaklarck added a commit that referenced this issue Dec 14, 2024
This includes non-existing variables in name, empty GROUP and even
missing END. We probably should validate syntax before even executing
tests/keywords and report syntax errors early. That should then be
done also with other control structures.
pekkaklarck added a commit that referenced this issue Dec 14, 2024
@pekkaklarck pekkaklarck added the acknowledge To be acknowledged in release notes label Dec 14, 2024
@pekkaklarck
Copy link
Member

This ought to be now done. The main functionality was provided in PR #5275 by @Snooz82 and I. I did some fixes and cleanup afterwards and as the last task wrote documentation. I hope @Snooz82 can review the docs and after that we can close this issue.

We still need to add GROUPsupport to the newly added JsonLogger, but that is part of #3423 instead of this issue.

pekkaklarck added a commit that referenced this issue Dec 16, 2024
JsonLogger is part of #3423 and the new GROUP syntax is #5257.
pekkaklarck added a commit that referenced this issue Dec 16, 2024
@pekkaklarck
Copy link
Member

I took another look at the docs and enhanced them a bit in the above commit. I believe the docs are good enough now and close this issue. If someone still wants to look them, you can find them here.

pekkaklarck added a commit that referenced this issue Dec 16, 2024
@pekkaklarck pekkaklarck changed the title GROUP syntax for grouping statements in Test|Task or Keywords. GROUP syntax for grouping keywords and control structures Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants