Closed Bug 1088666 Opened 10 years ago Closed 9 years ago

Re-structure the python marionette-client documentation

Categories

(Remote Protocol :: Marionette, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla36

People

(Reporter: ahal, Assigned: ahal)

References

Details

Attachments

(3 files, 1 obsolete file)

I find the documentation on http://marionette-client.readthedocs.org/en/latest/ a little hard to navigate for the following reasons:

1) Everything is on one page
2) Examples/usage/descriptive text and API references are interleaved together
3) Methods/attributes are not sorted alphabetically
4) Headers don't make it clear what objects/API's are under that section

I'd propose keeping the index page as is with the same headers, but move all the API docs to a new (or several new) "API Reference" page(s). On the index add more comprehensive examples to each sub-header with links to the relevant API docs.
I've been working on this here and there on the side.
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Here's my initial stab at this:
http://people.mozilla.org/~ahalberstadt/marionette-client-docs/html/

This is not a final version or anything. All feedback welcome and I have no qualms moving things around or re-writing sections if people don't like the layout/content. Also I still need to proof read it once more.

I'll push the change to reviewboard so people can raise issues/make comments more easily.
Attached file MozReview Request: bz://1088666/ahal (obsolete) —
/r/743 - Bug 1088666 - Re-organize marionette client's documentation into basic, advanced and api reference sections

Pull down this commit:

hg pull review -r ea1a523f550d3216e1f313cf55e6ad3b8049c520
https://reviewboard.mozilla.org/r/741/#review341

::: testing/marionette/client/docs/basics.rst
(Diff revision 1)
> +   client.switch_to_window(client.window_handles[1])

I might change this example to not depend on the order of handles.

::: testing/marionette/client/docs/basics.rst
(Diff revision 1)
> +   element = client.find_element('id', 'my-id')

Do we want to encourage the use of By.<foo> for this instead of strings like 'tag name'? I guess I've been using strings in the example tests.

::: testing/marionette/client/docs/advanced/findelement.rst
(Diff revision 1)
> +finding a DOM element on a webpage or in the chrome. Marionette provides

I might say "chrome UI", if that's accurate.

::: testing/marionette/client/docs/advanced/findelement.rst
(Diff revision 1)
> +:func:`~Marionette.find_elements`. Though some strategies are not implemented

Merge these sentences.

::: testing/marionette/client/docs/advanced/findelement.rst
(Diff revision 1)
> +* `tag name` - To find all the elements with a given tag, use `tag_name`::

s/tag_name/tag name/

::: testing/marionette/client/docs/advanced/findelement.rst
(Diff revision 1)
> +But this will raise an exception::

Maybe mention NoSuchElementException.

::: testing/marionette/client/docs/basics.rst
(Diff revision 1)
> +The Marionette python client library allows you to remotely control a

Is it worth explaining here a little bit about how the server runs in the browser/os, while the client is the end running our test programs? I found this confusing for a while, and I think it might be missing context to newcomers once we start talking about the server on the "Dealing with stale elements" page.

::: testing/marionette/client/docs/advanced/findelement.rst
(Diff revision 1)
> +The end result is that the DOM see's the widget as a single entity. It doesn't

s/see's/sees/
Attachment #8524687 - Flags: review?(dburns)
/r/743 - Bug 1088666 - Re-organize marionette client's documentation into basic, advanced and api reference sections
/r/843 - Fix chmanchester's review comments

Pull down these commits:

hg pull review -r d40a3b403e1979792b5a45b2a46325e11dcad89f
David, feel free to re-assign the review if you want. Or if someone else wants to review it, feel free to comment as well. I'd like to get this landed in time for the QA marionette training session during the work week if possible.

Also Malini had a suggestion of adding individual methods in the api reference to the side panel. I agree this is a good idea, but I can't seem to get it to work, likely because the methods are generated by autodoc. Sphinx can be a pain sometimes.. If someone else knows how to do this, please let me know!
https://reviewboard.mozilla.org/r/741/#review389

::: testing/marionette/client/docs/basics.rst
(Diff revision 2)
> +This returns an object listing the capabilities of the Marionette server. For

Returns a session id AND the capabilities of the marionette server

::: testing/marionette/client/docs/basics.rst
(Diff revision 2)
> +           with client.using_context(client.CONTEXT_CONTENT):

I don't think we need this `with` statement. `get_url()` and `client.switch_to_window(handle)` can be called from the content but actually manipulate the chrome. This is a special case because people want to switch top level browsing context if they hit a link that is target=\_blank

::: testing/marionette/client/docs/basics.rst
(Diff revision 2)
> +   element = client.find_element('id', 'my-id')

we should use the By enums for this instead of magic strings

::: testing/marionette/client/docs/interactive.rst
(Diff revision 2)
> +   first_link = client.find_element("tag name", "a")

By class enum here please

::: testing/marionette/client/docs/advanced/debug.rst
(Diff revision 2)
> +    elem = marionette.find_element('id', 'some-div')

By Class enum

::: testing/marionette/client/docs/advanced/findelement.rst
(Diff revision 2)
> +* `class name` - To find elements belonging to a certain class, use `class name`::

might be worth mentioning that find_element will return the first value from the DOM where find_elements finds all that match that value.

Also might be useful to have a note that find_element will raise an exception if no element is found. find_elements will just return an empty array
Comment on attachment 8524687 [details]
MozReview Request: bz://1088666/ahal

setting this to r- while you sort my comments so I dont get whined at by bugzilla ;)
Attachment #8524687 - Flags: review?(dburns) → review-
Attachment #8524687 - Flags: review- → review?(dburns)
/r/743 - Bug 1088666 - Re-organize marionette client's documentation into basic, advanced and api reference sections
/r/843 - Fix chmanchester's review comments
/r/941 - Fix :AutomatedTester's review comments

Pull down these commits:

hg pull review -r 4dc97864c6944d1b102ab7ab98287b480fe51ad7
Attachment #8524687 - Flags: review?(dburns) → review+
https://hg.mozilla.org/mozilla-central/rev/31b67a7052f4
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Attachment #8524687 - Attachment is obsolete: true
Attachment #8618454 - Flags: review+
Attachment #8618455 - Flags: review+
Attachment #8618456 - Flags: review+
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.