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

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

Merged
merged 1 commit into from
Jun 30, 2024

Conversation

RenanCarlosPereira
Copy link
Collaborator

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:

  • The unit tests use short timeouts on the cancellation token to trigger cancellation quickly.
  • They verify that the appropriate exception (TaskCanceledException) is thrown when cancellation is requested.
  • Ensures other rules that do not require cancellation complete without exceptions.

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
Copy link
Collaborator Author

@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
Copy link
Owner

asulwer commented Jun 30, 2024

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

@RenanCarlosPereira
Copy link
Collaborator Author

Nice, could you merge the PR, It's important to merge it always by another person 😀

Copy link
Owner

@asulwer asulwer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the additional tests

@asulwer asulwer merged commit 8ba9be2 into asulwer:main Jun 30, 2024
3 checks passed
@asulwer
Copy link
Owner

asulwer commented Jun 30, 2024

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

@asulwer
Copy link
Owner

asulwer commented Jun 30, 2024

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

@RenanCarlosPereira
Copy link
Collaborator Author

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

@asulwer
Copy link
Owner

asulwer commented Jul 1, 2024

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
Copy link
Collaborator Author

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
Copy link
Owner

asulwer commented Jul 1, 2024

thank you for the explanation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants