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

fix a signal creation bug #3973

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

robert-gurol-ebcont
Copy link

@robert-gurol-ebcont robert-gurol-ebcont commented Oct 4, 2024

Reopening #3920

@tijsrademakers I specifically asked for pointers in that pull request about where to add a unit test for this - I don't see through the big overall structure of the repository. I want to add unit tests, but I need your help to tell me what I can build on.

EDIT: The code was recently changed here, without any changes to unit tests - I really would have liked to know where to look for those 024ba64

fix a bug where for the second process
you start, signal processing would happen differently
(e.g. scope would always be null)
@tijsrademakers
Copy link
Contributor

Hi, I think a good place to look for is here https://github.com/flowable/flowable-engine/tree/main/modules/flowable-engine/src/test/java/org/flowable/engine/test/bpmn/event/signal. You can add it to the existing SignalEventTest unit test for example. Let me know if you need more info.

@robert-gurol-ebcont
Copy link
Author

Thank you! I'll have a look ❤️

PS: The existing tests still run with my change applied 🙈

@tijsrademakers
Copy link
Contributor

That's good that this test still runs. But it would be great if there could be a test added that's failing without your fix if possible.

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