mkorpela / pabot

Parallel executor for Robot Framework test cases.
https://pabot.org
Apache License 2.0
476 stars 152 forks source link

`DependencyLibrary` integration? #462

Open mentalisttraceur opened 2 years ago

mentalisttraceur commented 2 years ago

Figured I'd bring this up here to get wider discussion going.

A user of my DependencyLibrary opened this issue which got me thinking about how pabot and DependencyLibrary could combine great together.

I wrote a bunch of my thoughts in that issue, but the TL;DR of my current thoughts is:

  1. Seems to me like with some significant work inside DependencyLibrary, you could run your tests with pabot test-level parallelism and have it Just Work if only some of the tests need to be ordered sequentially.

  2. If it is possible for pabot to provide an exception that a test keyword can raise to say "please run/resume me after you've finished running {{specific test or suite}}", then it seems that DependencyLibrary can combine with pabot such that users get the most parallelism possible while keeping any test sequencing that they really need.

mentalisttraceur commented 2 years ago

I noticed pabot.SharedLibrary, and that seems like it will simplify work on the DependencyLibrary end.

But it looks like if the shared library is a Robot event listener, there's no event proxying. Unless I missed where that's implemented?

I'd like to get that implemented within pabot shared library, and I'm happy to do the work and open the PR myself if you'd like. I can also open another issue to discuss that specifically if you'd like.

mkorpela commented 2 years ago

Hi @mentalisttraceur , Thanks for interesting ideas. I'll need to spend a bit of time to understand the whole scope of your library and how to best approach this.

nestoracunablanco commented 2 years ago

Hi @mkorpela I am also interested on this feature. And I already took a look at the code and starting working on something. My suggestion will be, to use the .pabotsuite generated file, adding an extra keyword (for example: "#DEPENDS"), and this can be translated to "#WAIT" keyword afterwards. If you want I can make a draft pull request and work together on it. - right now is only a little bit of refactoring, in order to isolate the scope of the changes - Shall I make a fork and push the changes to my local copy or just pushing the new branch here?

nestoracunablanco commented 2 years ago

Hi @mkorpela here is the first draft regarding the proposal from 20 days ago. It is still somewhat far related to the last goal, which would involve some more related task scheduling. The trick for now is organizing the dependencies in different groups under the hood. Hope you like it.

mentalisttraceur commented 2 years ago

I haven't had much time to review this idea since I opened this issue in March, but I still think that the best (and very viable!) solution to this problem is:

  1. enrich pabot.SharedLibrary to also be a Robot event listener, and pass events to the library it wraps. Then:
    1. the entire ecosystem benefits: all libraries using Robot's event listener API start working with pabot.SharedLibrary, and incidentally
    2. users of both pabot and DependencyLibrary can just compose them by importing DependencyLibrary through pabot.SharedLibrary and the dependency tracking in DependencyLibrary works as normal.
  2. ideally, add a special exception subclass or API function to pabot, which tests can raise/call to tell pabot to rerun/resume that test when another some condition is true (this part might be hard - I haven't had time to look at the implementation, which is also why I haven't done anything on this issue). Then all sorts of libraries can be written or enhanced to dynamically provide ordering information to pabot, at runtime, such as DependencyLibrary.

I haven't looked at what @nestoracunablanco is doing in #479 besides the PR description, but maybe it is helpful along the way to achieving the second point, because once we have logic for dependencies, the next step is letting tests dynamically tell pabot dependency information at runtime, and having pabot react.

joonaskuisma commented 2 months ago

Hi @mkorpela, @nestoracunablanco and @mentalisttraceur. What is the status of this issue?

I'm working with DependencySolver which is given to robot as --prerunmodifier and it will be ready for release soon. I think it could be helpful in solving this problem. Let me give you an example of how it works. Consider the following simple test suite named as SuiteA.robot. I have defined dependencies between tests using DependencyLibrary only in [Setup] sections and as you can see, test C depends on B and B depends on A.

*** Settings ***
Library    DependencyLibrary

*** Test Cases ***
Test A
    [Tags]    A
    [Setup]    Log    message=Setup A
    Log    message=A

Test B
    [Tags]    B
    [Setup]    Run Keywords    Depends On Test    name=Test A
    ...    AND    Log    message=Setup B
    Log    message=B

Test C
    [Tags]    C
    [Setup]    Run Keywords    Depends On Test    name=Test B
    ...    AND    Log    message=Setup C
    Log    message=C

Now, if for example test A fails, DependencyLibrary will skip both test B and C, which is very nice. However, if we want to run only test C with the command robot --test "Test C" SuiteA.robot, it will not work, because only test C will be selected. If the dependency chain C => B => A is real, C will fail somewhere (Unlike in this simple example, which only gives WARN because of Dependency not met: test case 'Test B' not found. ).

This is where DependencySolver enters the picture. As a --prerunmodifier, it goes through all [Setup] sections and resolves dependencies. If we run the command robot --prerunmodifier DependencySolver:--test:"Test C" SuiteA.robot, DependencySolver can select the whole dependency chain (C plus both A and B). With robot this works very well.

I have also tested that, in principle, DependencySolver also works with the pabot --testlevelsplit --prerunmodifier DependencySolver:--test:"Test C" SuiteA.robot command. However, to run the execution order correctly, you also need to specify --ordering ordering.txt, which contains the following:

--test SuiteA.Test A
--test SuiteA.Test B #DEPENDS SuiteA.Test A
--test SuiteA.Test C #DEPENDS SuiteA.Test B

I could easily add a feature to DependecySolver that would write such a file. However, that alone is not enough, because:

  1. Argument --ordering ordering.txt will check and read file content before running --prerunmodifier. If you don't want to change --ordering implementation, could there be another argument that would try to read the content only after executing --prerunmodifier (That is, a little before pabot prints Storing .pabotsuitenames file text)?
  2. DependencySolver allows defining several dependencies, i.e. for example C => A and C => B, but so that B and A do not depend on each other. In this case, the contents of ordering.txt would be:
--test SuiteA.Test A
--test SuiteA.Test B
--test SuiteA.Test C #DEPENDS SuiteA.Test A #DEPENDS SuiteA.Test B

Would it be possible to change the implementation so that #DEPENDS could be list[str] instead of just str? (Note that DependencySolver will check all dependency chains so that they do not contain any kind of loops.)

  1. In addition, #DEPENDS should somehow identify what the status of dependencies is. That is, if A or B will fail, C would go into skip mode like DependencyLibrary does with robot. Apparently there are already some ideas for this?
mentalisttraceur commented 2 months ago

Hi again Joonas! Sorry I haven't had much capacity to think about this since we last talked. (I've even thought about just letting you take over DependencyLibrary. It has more potential than I can nourish right now.)

Re: 3: The idea I had was that if pabot.SharedLibrary was also an event listener (just a simple pass-through which propagated events to the wrapped library if it also had the relevant methods), then DependencyLibrary would automatically find out status of tests even if they execute in parallel Pabot test runners - no special coupling for #DEPENDS and status needed.

So your DependencySolver prerunmodifier pass would automatically generate the ordering, Pabot would ensure the ordering, and the enhanced pabot.SharedLibrary would pass the events through to DependencyLibrary (whose normal mechanisms for deciding whether dependencies have been skipped/failed should work fine once it has that info).

I honestly think the patch is mostly just copy-paste-rename of a thin wrapper event handler method which just checks if the same method exists on the wrapped library, and forwarding it. Should take less than a day, with a lot of that time being spent on reading the docs to see if anything has changed in robots event listener APIs and getting an exhaustive list. If we all agreed to be super nimble about it, we could just PR support for passing through only the couple of events that DependencyLibrary listens to, and say we'll add the rest later.

joonaskuisma commented 2 months ago

Hi @mentalisttraceur! Thanks for the answer! It would be ok for me to take responsibility for the DependencyLibrary.

I have thought that DependencySolver would be much more convenient as part of DependencyLibrary than as a separate package, which nevertheless needs DependencyLibrary for its operation. I suggest that I could do a PR for the DependencyLibrary on Github first. And then, if you have time to take a look and if necessary give feedback on it, I could for example be in the role of "Maintain" for the DependencyLibrary in the future. Then after DependencySolver robot integration is ready and working, I could work on this pabot integration issue. How would this sound?

nestoracunablanco commented 2 months ago

@joonaskuisma, I believe it would be feasible to modify the implementation of the #DEPENDS keyword to accept a list as input. Once you begin working on this, feel free to reach out to me for feedback, even (if you like) before creating a pull request. To maintain compatibility, we should also allow string inputs, which can be internally converted to a list containing a single element.

joonaskuisma commented 2 months ago

@nestoracunablanco Here is my initial solution for that: https://github.com/joonaskuisma/pabot/commit/fae2edaf6ac806d946e97c1e1f21decc258da113 It makes possible to use #DEPENDS keyword multiple times like this: --test Suite.C #DEPENDS Suite.A #DEPENDS Suite.B which means that test C depends both A and B. Then it is possible to run A and B parallel and when both of them are ready, run test C.

@mentalisttraceur I also tried to make pabot.SharedLibrary to working as you intended, but it did not working as I expected. If someone has a free time, help or ideas would be appreciated.

joonaskuisma commented 2 months ago

Because I couldn't get SharedLibrary to work properly with the import command Library pabot.SharedLibrary name=DependencyLibrary, I did the test using just pabotlib.

My resource.robot file looks like this:

*** Settings ***
Library    pabot.PabotLib

*** Keywords ***
Set Parallel Test Status
    [Documentation]    Used as test teardown
    Log    message=Setting status ${TEST_STATUS} to test ${TEST_NAME}.
    Set Parallel Value For Key    key=${TEST_NAME}    value=${TEST_STATUS}

Check Parallel Depends Status
    [Documentation]    Used in test [Setup] section
    [Arguments]    ${name}
    ${status}=    Get Parallel Value For Key    key=${name}
    IF  '${status}' != 'PASS'
        Skip    msg=${name} status was ${status}
    END

And suite_B.robot is like:

*** Settings ***
Resource    resource.robot
Test Teardown    Set Parallel Test Status

*** Test Cases ***
B1
    [Tags]    B1
    Sleep    1s
    Fail    msg=This fails

B2
    [Tags]    B2
    [Setup]    Check Parallel Depends Status    name=B1
    Sleep    2s
    Log    message=test

B3
    [Tags]    B3
    [Setup]    Check Parallel Depends Status    name=B2
    Sleep    3s
    Log    message=test

Suite_A.robot is almost identical but without the Fail keyword of the first test and of course all the B names have been changed to A.

ordering.txt file content is:

--test Data 2.suite A.A1
--test Data 2.suite A.A2 #DEPENDS Data 2.suite A.A1
--test Data 2.suite A.A3 #DEPENDS Data 2.suite A.A2
--test Data 2.suite B.B1
--test Data 2.suite B.B2 #DEPENDS Data 2.suite B.B1
--test Data 2.suite B.B3 #DEPENDS Data 2.suite B.B2

And then when I execute command pabot --testlevelsplit --pabotlib --ordering ordering.txt tests_own/data_2, test status values are shared between processes and tests B2 and B3 will be skipped as expected. So pabotlib and its keywords work fine :) Any ideas how to make the same functionality work with the Dependency Library? (i.e. how to make SharedLibrary work?)