asulwer / RulesEngine

Rules Engine with extensive Dynamic expression support
MIT License
19 stars 1 forks source link

Add Unit Tests for Cancellation Token Handling in ExecuteAllRulesAsync and ExecuteActionWorkflowAsync #31

Closed RenanCarlosPereira closed 3 months ago

RenanCarlosPereira commented 3 months ago

Description:

This pull request adds unit tests to verify the correct handling of cancellation tokens in the ExecuteAllRulesAsync and ExecuteActionWorkflowAsync methods of the Rules Engine.

Changes:

  1. New Unit Tests:

    • Added the test method ExecuteAllRulesAsync_WithCancellationToken_CancelsProperly to ensure the method respects the cancellation token and properly handles cancellation.
    • Added the test method ExecuteActionWorkflowAsync_WithCancellationToken_CancelsProperly to verify that executing a single rule with a cancellation token is handled correctly.
    • Both tests check that when the cancellation token is triggered, the appropriate exception (TaskCanceledException) is thrown.
  2. Custom Action Implementation:

    • Implemented ReturnTrueIfCancellationRequestedAction class to simulate an action that can be cancelled.
    • The action method checks for cancellation and returns true if the cancellation is requested.

Test Details:

Quick Tip:

In the library implementation, we are not breaking the loop if the cancellation is requested. This decision is left to the users of the library, allowing them to decide whether to stop execution using the cancellation flag or by throwing an exception. This approach provides more control to the final user, enabling them to complete tasks gracefully before breaking the process if they choose to.

Please review the changes and let me know if any adjustments are needed.

Thank you!

RenanCarlosPereira commented 3 months ago

@asulwer In this test I show how we can access the cancellation token without breaking changes.

users can cancel access to the cancellation token in the context.

check out the original documentation to see how actions work: https://microsoft.github.io/RulesEngine/#custom-actions πŸ™Œ

asulwer commented 3 months ago

I had some tests planned, but you beat me to it. thank you.

RenanCarlosPereira commented 3 months ago

Nice, could you merge the PR, It's important to merge it always by another person πŸ˜€

asulwer commented 3 months ago

oops! all PR's must have a branch associated with it. i didnt see a branch for this PR, did i miss it?

asulwer commented 3 months ago

and an open issue that can close when the merge is successful

RenanCarlosPereira commented 3 months ago

this PR didn't have a branch because I'm working on my fork and merging it into yours.

asulwer commented 3 months ago

it doesn't sound easy to test your PR. i do not really want to keep a copy of other developers forks just to test their PR, assuming it works that way.

RenanCarlosPereira commented 3 months ago

It works that way, I can open a branch in your fork, but just because Im a contributor, other people will have to fork them change whatever they are suggesting and then create a PR.

Thats the normal flow, only contributors can open branches in your fork πŸ˜…

asulwer commented 3 months ago

thank you for the explanation