Re: test for default history content

On Sat, Mar 29, 2014 at 1:24 PM, Jim Barnett <1jhbarnett@gmail.com> wrote:

>  Zjnue,
>  I agree that we need more tests, but why would defaultHistoryContent need
> to get passed to getTargetStates?  It's just an accessor function.  Though
> I do think we have to consider how getTransitionDomain and findLCCA should
> handle history states.  They ignore them at the moment, and _maybe_ that's
> ok...
>
> - Jim
>

Hi Jim,

Perhaps as quick background, my implementation follows (atm) the proposed
algorithm pretty much verbatim, in order to aid debugging for cases like
this and to make getting started with SCXML a little easier for myself and
others.

Simply put, the recently published changes to the algorithm for
defaultHistoryContent support are not satisfying test579.

While the getTargetStates function seems pretty harmless, it is also
hisory-aware and can redirect a transition target.
In the case of test579, the <initial> transition, targets <history
id="sh1">, but is effectively redirected to <state id="s01" >, skipping any
executable content of <history> along the way.

Please take a look at my code for getTargetStates and getTargetState below
[1].

It is only with the following two lines present in getTargetState:
                    if( defaultHistoryContent != null &&
h.parent.exists("id") )
                        defaultHistoryContent.set(h.parent.get("id"),
h.transition().next());
..that 579 passes.

As mentioned, I did not look at it in depth and will not be able to spend
more time on it over the next few days unfortunately.

So in summary, after adding the published additions to the algorithm,
test579 did not pass.
It was only after the changes to getTargetStates that it did.
Hope this helps for now.

Thanks,
Zjnue

[1]

    function getTargetStates( node : Node, ?defaultHistoryContent :
Hash<Iterable<Node>> ) : List<Node> {
        var l = new List<Node>();
        if( !node.exists("target") )
            return l;
        var ids = node.get("target").split(" ");
        var top = node;
        while( top.parent != null && !top.isTScxml() )
            top = top.parent;
        for( id in ids ) {
            var ts = getTargetState(top, id, defaultHistoryContent);
            for( tss in ts )
                l.add( tss );
        }
        return l;
    }

    function getTargetState( s : Node, id : String, ?defaultHistoryContent
: Hash<Iterable<Node>> ) : List<Node> {
        if( s.get("id") == id )
            return [s].toList();
        else {
            for( child in s.getChildStates() ) {
                var ss = getTargetState(child, id, defaultHistoryContent);
                if( ss != null )
                    return ss;
            }
            for( h in s.history() ) {
                var hh = getTargetState(h, id);
                if( hh != null ) {
                    if( defaultHistoryContent != null &&
h.parent.exists("id") )
                        defaultHistoryContent.set(h.parent.get("id"),
h.transition().next());
                    return historyValue.exists( h.get("id") ) ?
                        historyValue.get( h.get("id") ) :
                        getTargetStates( h.transition().next(),
defaultHistoryContent );
                }
            }
        }
        return null;
    }




>
> On 3/28/2014 8:51 PM, Zjnue Brzavi wrote:
>
>
>  I've added test579 to the suite.  Let me know if there are problems with
>> it or if you think there need to be more/different tests.--
>>
>
>  Hi,
>
>  Not had a very good look yet, but it appears we'll need to pass
> defaultHistoryContent to getTargetStates as well, as it is another area
> where the algorithm is skipping past some history elements without
> considering their executable content. This diff passes test579 :
> https://github.com/zjnue/hscxml/commit/4b9091470d3c7c1995e6b41900535e0437ec3eb4
>  Another test or 2 on the subject is probably a good idea.
>
>  Best regards,
>  Zjnue
>
>
> --
> Jim Barnett
> Genesys
>