latest version breaks autopilot-qt

[Michael Zanetti]

The latest update to xpathselect breaks autopilot-qt.

The issue is that the root node is created by reading the application name (which always has been that way) which can contain spaces. This hasn't been an issue so far, but only starts failing all the test with the latest upgrade of xpathselect.

[Thomi Richards (thomir-deactivatedaccount)]

Hi Michael,

This is indeed a change in behavior between the xpathselect versions, but only because the new version actually enforces node naming rules.

I looked in to the possibility of allowing spaces in nodes, but it makes the grammar much much more complicated - it introduces many edge cases that need to be explicitly caught in xpathsleect grammar.

Since this is really only a problem for the root node, I suggest the best place to plug this is in autopilot-qt itself. Perhaps for the root node you can translate "App Name" to "App_Name" or "AppName"?

[PS Jenkins bot (ps-jenkins)]

Fix committed into lp:autopilot-qt at revision 38, scheduled for release in autopilot-qt, milestone Unknown

[Michael Zanetti (mzanetti)]

The workaround comitted to autopilot-qt is not enough to fix issue properly. Besides autopilot-qt does not have a problem with spaces and dots.

Apparently it's autopilot itself that doesn't like it.

Will revert the workaround autopilot-qt once this is properly fixed.

[PS Jenkins bot (ps-jenkins)]

Fix committed into lp:autopilot-qt at revision 44, scheduled for release in autopilot-qt, milestone Unknown

[Thomi Richards (thomir-deactivatedaccount)]

Hi Michael,

I'm afraid I disagree with your assessment of this bug.

Autopilot has never allowed spaces in object names. Object names in the app need to be valid class names in Python. This has always been the case. The fact that the previous version of xpathselect allowed you to do this doesn't make it correct.

Think about it this way:

if you app exposes an object with an object name of "Foo.Bar", how are you going to instantiate that as a python type? Similarly, how would you instantiate an object with type "Foo Bar" ?

It's unfortunate that the previous version of xpathselect did not enforce these rules, but they still need to be enforced, and it's better to do it in xpathselect than on the client-side.

What exactly didn't work with your autopilot-qt patch?

[Thomi Richards (thomir-deactivatedaccount)]

Ahh, sorry - I may have mis-read your previous status change.

Is this fixed now? I'm marking this as "Won't fix" in autopilot, since the bug really isn't in AP itself... let me know if you think this is incorrect.

Cheers,

[Michael Zanetti (mzanetti)]

So where is the bug then? Its not xpathselect, its not autopilot and its not autopilot-qt. Yet it got introduced with an upgrade of libxpathselect.

All the tests involving only libxpathselect + autopilot-qt pass. As soon as autopilot gets involved it breaks. Autopilot-qt contains 2 very ugly workarounds for this now because we need autopilot to work or the phablet releasing will stall. In my opinion those workarounds shouldn't be there. If autopilot requires those workarounds, I think they should be in there and not distributed through the driver libs.

[Michael Zanetti (mzanetti)]

Regarding the "Fix committed", this was jenkins setting it to fix committed when I pushed the workaround int autopilot-qt.

What's not working still is if an app name contains dashes (I didn't work around that yet because we don't have any apps that do that) and all other special character that python does not allow (I don't know whats allowed in python and what itsn't).

Btw, here's the branch with tests that let you reproduce this easily:
https://code.launchpad.net/~mzanetti/autopilot-qt/add-tests/+merge/153695

Run "make && make check". All the tests with dots, spaces and dashes in the appname will pass. Then do a "cd tests/autopilot" and run the autopilot test suite in there. Tests containing the same app names will fail.