Remove dependency to rectangles.html in test_position.py

RESOLVED FIXED in Firefox 60

Status

P3
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: whimboo, Assigned: bjorn.arnelid, Mentored)

Tracking

(Blocks: 1 bug, {good-first-bug})

Version 3
mozilla60
good-first-bug
Points:
---

Firefox Tracking Flags

(firefox60 fixed)

Details

(Whiteboard: [lang=py])

User Story

For steps in how to get started please consult our documentation for new users:
https://firefox-source-docs.mozilla.org/testing/marionette/marionette/NewContributors.html

In the case of questions you can find us on IRC in the #ateam channel.

Attachments

(1 attachment)

The following test is using an external HTML testcase, which is simple enough to get integrated as data URL directly into the test. For Marionette unit tests we make use of the `inline` method. Here an example:

https://dxr.mozilla.org/mozilla-central/rev/a928be5dacc3b544e29c0612b3f8cda6447df802/testing/marionette/harness/marionette_harness/tests/unit/test_typing.py#14

The same method should be applied to the following test:

https://dxr.mozilla.org/mozilla-central/rev/default/testing/marionette/harness/marionette_harness/tests/unit/test_position.py

By adding as minimal necessary code from the following HTML testcase:

https://dxr.mozilla.org/mozilla-central/rev/default/testing/marionette/harness/marionette_harness/www/rectangles.html

The HTML testcase should be deleted afterward.
No longer depends on: 1423661
Blocks: 1423638
(Assignee)

Comment 1

a year ago
I would like to contribute to this bug. I will send a patch as soon as i have figured out what to do in mercurial.
Thanks. If you struggle don't hesitate to ask here on Bugzilla, or in #ateam on irc.mozilla.org.
Assignee: nobody → bjorn.arnelid
Status: NEW → ASSIGNED

Comment 4

a year ago
mozreview-review
Comment on attachment 8948974 [details]
Bug 1434909 - Remove dependency to rectangles.html in test_position.py

https://reviewboard.mozilla.org/r/218392/#review224172

Stealing this review.

::: testing/marionette/harness/marionette_harness/tests/unit/test_position.py:14
(Diff revision 1)
> -class TestPosition(MarionetteTestCase):
> +def inline_rectangle():
> +    inline =  """
> +    <head>

Because this is only used once, can you create a function called
inline that takes a string as input, then move the multi-line string
into the test?

::: testing/marionette/harness/marionette_harness/tests/unit/test_position.py:18
(Diff revision 1)
>  
> -class TestPosition(MarionetteTestCase):
> +def inline_rectangle():
> +    inline =  """
> +    <head>
> +        <title>Rectangles</title>
> +        <style type="text/css">

type="text/css" is unnecessary.

::: testing/marionette/harness/marionette_harness/tests/unit/test_position.py:42
(Diff revision 1)
> +    """
> +    return "data:text/html;charset=utf-8,{}".format(urllib.quote(inline))
>  
> +class TestPosition(MarionetteTestCase):
>      def test_should_get_element_position_back(self):
>          test_url = self.marionette.absolute_url('rectangles.html')

test_url is not used anymore.

::: testing/marionette/harness/marionette_harness/tests/unit/test_position.py:42
(Diff revision 1)
> +    """
> +    return "data:text/html;charset=utf-8,{}".format(urllib.quote(inline))
>  
> +class TestPosition(MarionetteTestCase):
>      def test_should_get_element_position_back(self):
>          test_url = self.marionette.absolute_url('rectangles.html')

rectangles.html is only used in this test, which means the fixture
file can be removed.
Attachment #8948974 - Flags: review-
Comment hidden (mozreview-request)

Comment 6

a year ago
mozreview-review
Comment on attachment 8948974 [details]
Bug 1434909 - Remove dependency to rectangles.html in test_position.py

https://reviewboard.mozilla.org/r/218392/#review224186

Looks OK now, but triggered a try run to be sure: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dd4aa813f342
Attachment #8948974 - Flags: review+
Next time, please feel free to flag me with r?ato in the commit message.
(Assignee)

Comment 8

a year ago
I will remember to do that.

Comment 9

a year ago
Pushed by atolfsen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/042448262514
Remove dependency to rectangles.html in test_position.py r=ato

Comment 10

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/042448262514
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox60: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.