[Laszlo-dev] [Platform-team] testing

[P T Withington]

On 2008-12-02, at 13:15EST, André Bargull wrote:

> On 12/2/2008 6:45 PM, P T Withington wrote:
>> I'm all for doing the right thing, but not for doing it randomly.   
>> It seems to me we ought to be consistent and always set the pointer  
>> to null if you delete it.
>
> The next sibling is only selected if "rerunxpath" is set to `false`,  
> which is not the default for datapaths (but for datapointers!).
> On a normal datapath, if you delete a node, the xpath will simply be  
> re-evaluated.

I'm not the datapath expert, but that seems broken.  If I don't have  
rerunxpath, then to me it seems that deleting the node I am pointing  
at ought to leave me pointing at nothing.  If rerunxpath is on, then  
it will simply depend on whether or not my xpath has a match after the  
deletion.  As you said in your subsequent message the 'delete and  
next' behavior makes no sense for complex xpaths.  The more I think  
about it, this behavior looks like it was not well thought out.  I  
think we ought to remove it.

> If the old behavior is needed, then maybe we need
>> deleteAndNext (and deleteAndPrevious) API's, or to memoize  
>> sufficiently to allow you to do next/previous after the delete.
>
> Why not adding an argument to "deleteNode" instead of new methods?

I'd add nothing at all if this extension is not needed.  If it is a  
common operation, then we should discuss how to add it.

>> On 2008-12-02, at 12:11EST, Henry Minsky wrote:
>>> I would lean towards making it become null, because I like to have
>>> "less magic" when possible.
>>>
>>> But I don't feel too strongly about it in this case. David Temkin
>>> tended to like to have a little more "magic" in the language, to  
>>> make
>>> it easier for people to put apps together faster with
>>> less code, but I find it easier when learning an API if the methods
>>> have simpler behaviors with less special cases, which are are easy  
>>> to
>>> remember and describe.
>>>
>>>
>>> Keeping the old
>>> behavior would also be one less thing to worry about to make life
>>> easier for people porting  their existing apps to 4.2.
>>>
>>>
>>>
>>>
>>>
>>>
>>> On Tue, Dec 2, 2008 at 11:26 AM, André Bargull <andre.bargull at udo.edu 
>>> > wrote:
>>>> If you delete a node (call it "A") through  
>>>> "LzDatapointer#deleteNode()" and
>>>> if this node has got a next sibling (call it "B"), the  
>>>> datapointer will
>>>> delete "A" and then select "B".
>>>> But if "A" has no next sibling, but a previous sibling (call it  
>>>> "C"), the
>>>> changed method will select "C" instead, but before the pointer  
>>>> was set to
>>>> `null`.
>>>> I said it is more reasonable to select any possible sibling (next  
>>>> _and_
>>>> previous), but the testcases seem to require that _no_ previous  
>>>> sibling is
>>>> selected. Changing the testcases to match the new semantics is  
>>>> surely
>>>> possible, but maybe there is some reason not to select the  
>>>> previous sibling.
>>>> Any ideas?
>>>>
>>>>
>>>> Ok, here's a simple example which will work in trunk-nightly.
>>>>>
>>>>> <canvas debug="true" >
>>>>> <dataset name="ds" >
>>>>>  <items>
>>>>>    <item id="a" />
>>>>>    <item id="b" />
>>>>>  </items>
>>>>>  <items>
>>>>>    <item id="c" />
>>>>>    <item id="d" />
>>>>>  </items>
>>>>> </dataset>
>>>>>
>>>>> <handler name="oninit" >
>>>>>  var dp1 = ds.getPointer();
>>>>>  dp1.setXPath("/items[1]/item[1]");
>>>>>  Debug.write("valid = %w", dp1.isValid());
>>>>>  dp1.deleteNode();
>>>>>  Debug.write("valid = %w, id='%s' (expect '%s')",       
>>>>> dp1.isValid(),
>>>>> dp1.getNodeAttribute("id"), "b");
>>>>>
>>>>>  var dp2 = ds.getPointer();
>>>>>  dp2.setXPath("/items[2]/item[2]");
>>>>>  Debug.write("valid = %w", dp2.isValid());
>>>>>  dp2.deleteNode();
>>>>
>>>>>   // this will work in trunk-nightly, but not in earlier versions
>>>>>
>>>>>  Debug.write("valid = %w, id='%s' (expect '%s')",       
>>>>> dp2.isValid(),
>>>>> dp2.getNodeAttribute("id"), "c");
>>>>> </handler>
>>>>> </canvas>
>>>>
>>>>
>>>>
>>>>
>>>> On 12/2/2008 5:10 PM, Henry Minsky wrote:
>>>>>
>>>>> Oh sorry, I should have sent this to laszlo-dev....
>>>>>
>>>>>
>>>>> On Tue, Dec 2, 2008 at 11:07 AM, André Bargull <andre.bargull at udo.edu 
>>>>> >
>>>>> wrote:
>>>>>>
>>>>>> [I've removed the cc for "Platform Team
>>>>>> <platform-team at laszlosystems.com>"
>>>>>> because I haven't got access to that mailing-list]
>>>>>>
>>>>>> I'll track it down and check the assertions in the test..
>>>>>>
>>>>>>
>>>>>> On 12/2/2008 5:02 PM, André Bargull wrote:
>>>>>>>
>>>>>>> Ah, ok. The last change from Josh to  
>>>>>>> "LzDatapointer#deleteNode()" is the
>>>>>>> culprit. So actually my fault, since I've proposed that change  
>>>>>>> and
>>>>>>> approved
>>>>>>> the change, too...
>>>>>>>
>>>>>>>
>>>>>>> On 12/2/2008 4:41 PM, Henry Minsky wrote:
>>>>>>>>
>>>>>>>> No, actually I did a clean build in a clean top-of-tree trunk  
>>>>>>>> and I
>>>>>>>> still get those errors.  Looks like a real regression
>>>>>>>> from something. I think I will try max's binary search tool  
>>>>>>>> to find
>>>>>>>> which revision this happened in.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Dec 2, 2008 at 10:35 AM, Henry Minsky
>>>>>>>> <hminsky at laszlosystems.com>
>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> Actually, it looks like it's that changeset I just sent out  
>>>>>>>>> for the
>>>>>>>>> datapath sortorder is causing a failure, it seems like it's  
>>>>>>>>> only
>>>>>>>>> happening in my workspace that has that modification.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Tue, Dec 2, 2008 at 10:23 AM, André Bargull <andre.bargull at udo.edu 
>>>>>>>>> >
>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>> That's the cachebreak-test, you need to run alldata another  
>>>>>>>>>> time,
>>>>>>>>>> then
>>>>>>>>>> it
>>>>>>>>>> should work.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 12/2/2008 4:12 PM, Henry Minsky wrote:
>>>>>>>>>>>
>>>>>>>>>>> speaking of which , I am seeing these errors in alldata.lzx
>>>>>>>>>>>
>>>>>>>>>>> Tests: 325 Failures: 3 Errors: 0
>>>>>>>>>>> TestFailure: test2 failed: False:  expected false got true
>>>>>>>>>>> TestFailure: test2 failed: False:  expected false got true
>>>>>>>>>>> TestFailure: test failed: False:  expected false got true
>>>>>>>>>>>
>>>>>>>>>>> I'm tracking down which test it is now. I guess we ought  
>>>>>>>>>>> to be
>>>>>>>>>>> printing out the name of
>>>>>>>>>>> the test suite class that is running to make clear which  
>>>>>>>>>>> test suite
>>>>>>>>>>> is
>>>>>>>>>>> failing.
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> -- 
>>>>>>>>> Henry Minsky
>>>>>>>>> Software Architect
>>>>>>>>> hminsky at laszlosystems.com
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>>
>>> -- 
>>> Henry Minsky
>>> Software Architect
>>> hminsky at laszlosystems.com
>