-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[v5] Enable additional SCXML tests #1240
Conversation
🦋 Changeset is good to goLatest commit: 6043a1c We got this. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 6043a1c:
|
@Andarist Is this one good to go? |
`Unable to send event to child '${to}' from service '${this.id}'.` | ||
); | ||
this.send( | ||
toSCXMLEvent<TEvent>(actionTypes.errorExecution, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically if we want to match SCXML this should be error.communication
:
If the sending SCXML session specifies a session that does not exist or is inaccessible, the SCXML Processor must place the error error.communication on the internal event queue of the sending session.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not exactly; in this case, it's that the target
is invalid, which should raise error.execution
instead. See test194.txml.scxml
:
<?xml version="1.0" encoding="UTF-8"?><!-- we test that specifying an illegal target for <send> causes the event error.execution to be raised. If it does,
we succeed. Otherwise we eventually timeout and fail. --><scxml xmlns="http://www.w3.org/2005/07/scxml" xmlns:conf="http://www.w3.org/2005/scxml-conformance" initial="s0" version="1.0" datamodel="ecmascript">
<state id="s0">
<onentry>
<!-- should cause an error -->
<send target="baz" event="event2"/>
<!-- this will get added to the external event queue after the error has been raised -->
<send event="timeout"/>
</onentry>
<!-- once we've entered the state, we should check for internal events first -->
<transition event="error.execution" target="pass"/>
<transition event="*" target="fail"/>
</state>
<final id="pass"><onentry><log label="Outcome" expr="'pass'"/></onentry></final>
<final id="fail"><onentry><log label="Outcome" expr="'fail'"/></onentry></final>
</scxml>
That test will fail if I change it to error.communication
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. Not sure when error.communication
would ever be dispatched then 🤷♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. Not sure when error.communication would ever be dispatched then 🤷♂️
I think once we get into the XState-on-the-server side of things, error.communication
will be more relevant. E.g., a remote statechart might exist, but can return error codes if it's down. Or the ActorRef
can indicate when communicating with it failed.
Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, it would just be great to create a ticket for adding tests for & co as per #1240 (comment)
I forgot - would be great to create a changeset about user-facing changes. |
No description provided.