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

additional Testing with tests derived from xstate.core 4.25 #49

Open
wants to merge 69 commits into
base: master
Choose a base branch
from

Conversation

auphofBSF
Copy link

THIS PR is WIP - it introduces many additional test and functions base on xstate.core (main) statelyai/xstate@019f849

I am pushing it as a WIP PR for visibility, comment and contribution.
I have in many instances pulled the complete TS module in as comments or multiline strings (They fold nicely). I have in most instances left the xstate.core ts code in as comments for ease of verification.

This PR is In line with needs for expanding testing, docstrings and particularly attempting to get history functioning

I am working through testcases and introducing code that is necessary to get particular test cases running with code as closely aligned to xstate.core

Areas being worked on.

Notes on Machine config from JS snippet

I have added to Machine __init__ to take a string such that a JS snippet of a machine config just works (exploring this use of js2py further, but interestingly even a simple transition condition in JS works (NOT suggesting this should be used in PRODUCTION).
This enables the direct copy and paste from JS xstate.core machine/createmachine or from the xstate-viz and avoids having to recode into pythonic dict structures, particularly useful in test case copy from xstate.core
Example:

xstate_js_config = '''{
                id: 'fan',
                initial: 'off',
                states: {
                    off: {
                    on: {
                        POWER:'on.hist',
                        H_POWER:'on.H',
                        SWITCH:'on.first'
                    }

                    },
                    on: {
                    initial: 'first',
                    states: {
                        first: {
                        on: {
                            SWITCH:'second'
                        }
                        },
                        second: {
                        on: {
                            SWITCH: 'third'
                        }
                        },
                        third: {},
                        H: {
                        type: 'history'
                        },
                        hist: {
                        type: 'history',
                        history:'shallow'
                        },
                    }
                    }
                }
        }
        '''
machine = Machine(xstate_js_config)

# or alternatively convert to python machine config

config = get_configuration_from_js(xstate_js_config)

Because of the breadth of testing updates and other issues are being touched on, my attempts may not be inline with suggestions and discussions particularly in the Issues listed below, I am however open to suggestions and alternative ways. It would be just great to get similar functionality in a python environment as is currently available in TS or JS

Following issues will have some cross over, particularly note @mr-bjerre and @nobrayner being author of many issues and contributors . and of course @davidkpiano

Other relevant issues:

docs: issues, todos and docstrings

the config as snippet of javascript allows
easy copy and paste from visualizer
may not handle all especially where functions are defined
see `test_state.py`  where execeptions
are handled `machine_xstate_js_config`
docs: issues, todos and docstrings
chore: ensure repr outputs have class and add some
attributes, explore __str__
fix :get_configuration_from_js
feat: varous statepath functions ---- wip
fix: get_configuration_from_state --- wip
feat: state.__str__ for tests
with no id in config Nodes where being created with
(machine).(machine) id's
feat: support for history and transition - wip

implement much support for history and align code to JS xstate.core
a condition in a JS snippet evaluates and passes
TODO added for python callable test
some scxml failing, will resolve in next commit
support functions for this test
@davidkpiano
Copy link
Member

Thanks for this! I will be reviewing this soon, unless you want me to wait until it's ready.

@auphofBSF
Copy link
Author

Thanks for this! I will be reviewing this soon, unless you want me to wait until it's ready.

Hey @davidkpiano , a general lookaround and overview would be good, I am now getting into a cycle of commits around individual tests or sets of tests, but even these carry many supporting core implementations.

Items to think about

Some issues that have been tripping me up are

  • cyclical dependendencies the many from xstate.xxxx import yyyy I made a large set of commits to try and solve this c327e8a, but may need to look at overall structure more clearly

  • Types (or DucK Typing as is in Python) , what to do with types to support vscode intellisense some structure and verification.

General Goal

To get History working against the test cases in xstate.core, then would probably move this PR from Draft to Candidate or else one can go on for ever.

I currently have got StateNode. Transitions test cases e2c39b0 and d45493d working and a few other test cases

@davidkpiano
Copy link
Member

There's a lot to go through here, but one thing I'm seeing that we should keep in mind is that this should not be a one-to-one reimplementation of XState v4 (JS), but rather just have the same core surface area API. That's why some of the data structures and algorithms are very different from it; there's no need to copy everything over.

What are your thoughts on that?

@auphofBSF
Copy link
Author

That is a very fuzzy line, I have selected some of the XState v4 (JS) core test cases and working back from passing those tests.

The result is that xstate-python requires adding into. Instead of re inventing it seem to make sense to use the XState v4 (JS) and keep as close to it as possible for maintaining 1 consistent logic across JS and Python.

So to define what is the Core Surface API, I have for the moment only looking for history and by association the state transitions as seen as some of the core functionality of immediate interest

@davidkpiano
Copy link
Member

That is a very fuzzy line, I have selected some of the XState v4 (JS) core test cases and working back from passing those tests.

The result is that xstate-python requires adding into. Instead of re inventing it seem to make sense to use the XState v4 (JS) and keep as close to it as possible for maintaining 1 consistent logic across JS and Python.

So to define what is the Core Surface API, I have for the moment only looking for history and by association the state transitions as seen as some of the core functionality of immediate interest

It's better to look at the v5 branch for this: https://github.com/statelyai/xstate/tree/next

fix : error on unsupported events

the above is adding support towards eventless transitions
includes fixes to algorithm incorrectly copied from XstateJS
Create a full State object if a state is passed in form of a dict
WIP apart from 1st test ,
tests are failing for apparent algorithim issues
fix:  _get_relative handling nested key
fix: to_state_path  incorrectly implemented from original JS
fix: remove resolve_history workaround with to_state_paths

history type is often not set for history deep or shallow
automatically infer  history type if that is the case

 _get_relative was not handling a  long key ie `on.A.hist`
fix: to_state_path  incorrectly implemented from original JS

whilst passing many history tests, it is failing
on 4 parallel hist tests
Test failing, fixes to be in additional commit
a prior `state.history_value` was being updated
this is now imutable
fix: relative node not being handled properly
test: history check state.history_value is not changed
test: workaround for test env detection
in resolving history tests , the get_from_relative was not correctly
handling statee_id's ie `#d..e
history uniti tests, particularly `TestParallelHistoryStatesHistory`
require state tree branches not to searced for states to enter if
history is present and resolvable
@auphofBSF
Copy link
Author

Have resolved all History tests from XstateJS that can be supported, only 3 required to be skipped. 🎉

I believe this PR is as far as I want to take it before further review. So I am removing the draft status

=========================== short test summary info ============================
SKIPPED [1] tests/test_history.py:244: Eventless Transition `Always`, not yet implemented
SKIPPED [1] tests/test_history.py:311: interpreter, not yet implemented
SKIPPED [1] tests/test_history.py:978: Transient `always` not implemented yet
SKIPPED [1] tests/test_state.py:164: Not implemented yet
SKIPPED [1] tests/test_state.py:178: Not implemented yet
SKIPPED [1] tests/test_state.py:192: Not implemented yet
SKIPPED [1] tests/test_state.py:672: Not implemented yet
SKIPPED [1] tests/test_state.py:684: Not implemented yet
SKIPPED [1] tests/test_state.py:728: Not implemented yet
SKIPPED [1] tests/test_state.py:782: Not implemented yet
SKIPPED [1] tests/test_state.py:793: Not implemented yet
SKIPPED [1] tests/test_state.py:816: Not implemented yet
SKIPPED [1] tests/test_state_in.py:230: Transition based on Orthogonal/Parallel or Hierachical relative states, not yet implemented
SKIPPED [1] tests/test_state_in.py:344: Transition Guards, not yet implemented
SKIPPED [1] tests/test_state_in.py:387: Transition Guards, not yet implemented
66 passed, 15 skipped in 37.47s

@auphofBSF auphofBSF marked this pull request as ready for review October 20, 2021 22:01
@davidkpiano
Copy link
Member

@auphofBSF This will take me some time to get through, but just for my understanding, is this trying to be a one-to-one backport of XState v4? That should not be the goal of XState Python, and if that's the case, there's a lot of things that we can omit here.

@auphofBSF
Copy link
Author

@auphofBSF This will take me some time to get through, but just for my understanding, is this trying to be a one-to-one backport of XState v4? That should not be the goal of XState Python, and if that's the case, there's a lot of things that we can omit here.

No the goal was not to be a back port of XState V4, the goal was to get history working and add tests and get a deeper understanding of the functionality of Xstate,

In using XstateV4 tests as validation, I used XStateJS V4 code as a known and validated implementation, Where I was having difficulty I could step through and understand the JS implementation and get it working in Python.

I would Like to have used XstateJS V5 but I had already implemented significant parts when you suggested I look at it and just needed to get history working. I believe I only implemented methods and attributes relevant to getting history working, but welcome the deeper look into it.

@auphofBSF
Copy link
Author

It may be worth considering refactoring xstate-python to a code base/ Interfaces closer in line with XStateJS V5 particularly from a point of view of Validation, Documentation and long term support.

If there is a desire to resolve some of the functional limitations discovered and the issues subsequently raised such as #50,#51 #52. then possibly these can be the catalyst to implement a subset of interfaces that can make use of common validation and documentation with XstateJS.

I did not want to try and resolve all functional limitations in this PR as I don't have a full vision of what Xstate-Python wants to be.

I certainly encountered difficulty with divergence from XstateJs as much of the xstate-python core implemented 2 years ago appeared to be from a different code base with resultant Interfaces quite different to what currently exists in XstateJS V4 or V5.

My particular use and understanding of how we want to use Xstate-Python is still very much emerging and exploratory but I hope I can contribute usefully to getting the great stuff I see working in JS also available for Python

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.

3 participants