Merge lp://staging/~canonical-platform-qa/autopilot/fix-cpo-having-different-name-1337004 into lp://staging/autopilot
Proposed by
Christopher Lee
| Status: | Merged |
|---|---|
| Approved by: | Christopher Lee |
| Approved revision: | 564 |
| Merged at revision: | 567 |
| Proposed branch: | lp://staging/~canonical-platform-qa/autopilot/fix-cpo-having-different-name-1337004 |
| Merge into: | lp://staging/autopilot |
| Diff against target: |
168 lines (+110/-2) 4 files modified
autopilot/introspection/dbus.py (+38/-1) autopilot/tests/functional/test_input_stack.py (+1/-1) autopilot/tests/functional/test_introspection_features.py (+29/-0) autopilot/tests/unit/test_introspection_dbus.py (+42/-0) |
| To merge this branch: | bzr merge lp://staging/~canonical-platform-qa/autopilot/fix-cpo-having-different-name-1337004 |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| PS Jenkins bot | continuous-integration | Approve | |
| Max Brustkern (community) | Approve | ||
|
Review via email:
|
|||
This proposal supersedes a proposal from 2015-05-21.
Commit message
Allow Custom Proxy Object classes to have a different name to that of the underlying UI Type
Description of the change
Allow Custom Proxy Object classes to have a different name to that of the underlying UI Type (I.e. having a RedRect CPO for a QQuickRectangle).
To post a comment you must log in.
Revision history for this message
| Thomi Richards (thomir-deactivatedaccount) wrote | # |
Revision history for this message
| PS Jenkins bot (ps-jenkins) wrote | # |
review:
Needs Fixing
(continuous-integration)
Revision history for this message
| PS Jenkins bot (ps-jenkins) wrote | # |
review:
Approve
(continuous-integration)
Revision history for this message
| PS Jenkins bot (ps-jenkins) wrote | # |
review:
Approve
(continuous-integration)
Revision history for this message
| PS Jenkins bot (ps-jenkins) wrote | # |
review:
Approve
(continuous-integration)
Revision history for this message
| Nicholas Skaggs (nskaggs) wrote | # |
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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_
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...