Repository navigation
fix: register one event per id a prefixed attribute expands to - #675
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #675 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 52 52
Lines 5672 5685 +13
Branches 891 896 +5
=========================================
+ Hits 5672 5685 +13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
`_expand_event_id` turned `error_foo` into the string `"error_foo error.foo"`,
and the callers registered a single event under that glued id. The transition
carried both ids, so `send("error.foo")` worked, but `sm.error_foo()` raised
`TypeError` on a bare transition and was a no-op on an `Event(...)`, and
`allowed_events` / `enabled_events()` raised `AttributeError` looking up
`error.execution` on the instance. Inside a nested state body the glued id
was split by `Events.add`, which dropped the `name`, `delay` and `internal`
declared on the `Event`. The `@source.to(target)` decorator form never
expanded at all.
The expansion now returns the ids as a list and each call site, including the
decorator form, registers one event per id, binding each as a class attribute.
An `Event()` declared without transitions and referenced through `event=` maps
to every id it expands to, so `_events_to_update` keeps a list of replacements
per placeholder and looks the placeholder up by membership rather than by the
prefix matcher, which crashed on a `"*"` transition. `AnyState` adds every id
to a single copy per state instead of copying the transition once per id.
Fixes #654. Also fixes question 1 of #644.
Signed-off-by: Fernando Macedo <fgmacedo@gmail.com>
fgmacedo
force-pushed
the
fgmacedo/fix-654-expanded-event-ids
branch
from
October 13, 2026 13:12
17c6d51 to
7ccf95f
Compare
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Why
_expand_event_id("error_foo")returned the glued string"error_foo error.foo"and the chart registered 1 event under it instead of 2, sosm.error_foo(),allowed_eventsandenabled_events()were broken for everyerror_/done_state_/done_invoke_attribute (#654, question 1 of #644).Fixes #654. Also fixes question 1 of #644 (left open for its question 2).
What changes
sm.error_foo()runs the transition instead of raisingTypeError(bare) or doing nothing (Event(...)).allowed_eventsandenabled_events()list both ids instead of raisingAttributeErroronerror.execution, on both engines.name,delayandinternaldeclared on anEvent(...)inside a nested state body are kept on both ids instead of dropped.@source.to(target)decorator form expands too; it registered only the attribute name before.from_.any()with a prefixed name copies its transition once per state carrying both ids, instead of once per id (one diagram edge per state, callbacks registered once).Event(...)attribute plus a"*"transition builds instead of crashing withValueError: list.remove(x); placeholders are now found by membership, not by the prefix matcher.Evidence
Before, on
origin/main, with theErrorLoggerchart from #644:After:
New tests:
tests/test_expanded_event_ids.py(3 prefixes x 4 declaration forms x top-level/nested, sync and async, plusfrom_.any()), 2 cases intests/test_events.py, and the invertedtest_expanded_id_keeps_the_declared_arguments_on_both_ids. Run against theorigin/mainpackage, 109 of the new cases fail for the reported reasons.Not verified
MachineMixin/ Django models with prefixed events (no code path there touches registration, but not exercised).Event()placeholder referenced from inside a nested state body is still not replaced (_update_event_referenceswalks only top-level states); pre-existing, skipped in the new tests with that reason.on_error_executionnaming convention still only fires for a trigger literally namederror_execution, not for the engine-raisederror.execution; pre-existing, left for a follow-up.