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 event count for multiple handlers #556

Merged
merged 4 commits into from
Feb 21, 2024

Conversation

hwuethrich
Copy link
Contributor

If multiple event handlers are registered for the same event, the counter is incremented before each handler. This results in filters like every: 10 to never match any events.

If multiple event handlers are registered for the same event, the counter is incremented before each handler. This results in filters like `every: 10` to never match any events.
@seanmor5
Copy link
Contributor

Good catch, thanks!

@josevalim
Copy link
Contributor

Could we have a test somehow? 🤔

@hwuethrich
Copy link
Contributor Author

hwuethrich commented Feb 20, 2024

@josevalim I'm currently testing it by always adding a second handler in the dummy loop, which causes the existing tests to fail without the fix.

Maybe not the most elegant/obvious solution. I'm happy to refactor the test if you point me in the right direction (I'm very new to axon) 😅

@hwuethrich
Copy link
Contributor Author

I've added a test for the event counting. Let me know if this is OK or if you would like me to further refactor the test.

@hwuethrich
Copy link
Contributor Author

Actually, the test now passes even without the fix because I removed the dummy handlers. Let me re-add them again...

@seanmor5 seanmor5 merged commit 885d2d2 into elixir-nx:main Feb 21, 2024
3 of 5 checks passed
@hwuethrich hwuethrich deleted the fix_event_count branch February 21, 2024 13:17
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.

None yet

3 participants