Code review comment for lp://staging/~canonical-platform-qa/autopilot/fix-cpo-having-different-name-1337004

Hi,

You asked for my feedback, so.... brace yourself :D

First, I think this will solve your immediate problem, so don't take anything else I write here as an immediate disapproval. However, I think you're painting yourself into a corner API-design-wise, and it can be avoided.

Let's consider the 60k ft view:

* Test authors want to select one or more objects from the object tree.
* From a data flow POV, that looks like:

 test query (in python)
   -> xpathselect query string (over dbus)
   -> libxpathselect
   -> query data structure (over dbus)
   -> proxy objects (in python)

Currently we give test authors a way to filter what proxy objects get created from the query data structure (via validate_dbus_object), and the long-standing bug you're trying to solve is that the API for generating the query is very limiting on the client-side.

Your solution below is to allow test authors to override the class name generated in the source query.

I think that doesn't go far enough.

Since autopilot 1.5 was released, we have a pretty good encapsulation of the xpathselect query protocol. Why not make the tweaks necessary so that test authors can:

 - create any valid xpathselect query object
 - filter the results as they come back
 - tell autopilot to create proxies with any given class or class mixin

The big, fundamental mistake we made in autopilot was to conflate "a connection to an application" with "a set of classes that can be used to represent application internals". I think that separating those wouldn't be too hard.

The specific issue I see with this merge proposal is, it'll only be a matter of time before Leo (sorry Leo, I'm totally picking on you here) says "I want to write a test where I only select QQuickRectangles that have property X set to Z, and filtering them on the client side is too costly". That's a valid use case, and the only way you'll be able to support that is by further extending an already bloated API.

I suggest you think about how you can separate the concerns mentioned above, and start writing a new query API, while leaving the old API in place for a release to give people time to migrate (hell, you can probably make it 99% backwards compatible).

Let me know if anything doesn't make sense. I'm not subscribed to this MP, so if you reply, you'll need to ping me on IRC...