Andreas Tolfsen ❲:ato❳ Assignee
	Description • 10 years ago

Tests are currently doing self.assertTrue("foo" in bar) whereas self.assertIn("foo", bar) would be preferable for better debugging when a test fails.

	Andreas Tolfsen ❲:ato❳ Assignee
	Comment 1 • 10 years ago

Must be landed after bug 977893.

	David Burns :automatedtester
	Comment 2 • 10 years ago

Comment on attachment 8392968 [details] [diff] [review]
0001-Bug-984921-Replace-assertTrue-foo-in-bar-in-test_nav.patch

Review of attachment 8392968 [details] [diff] [review]:
-----------------------------------------------------------------

r+ can be carried forward with cleanup of nits

::: testing/marionette/client/marionette/tests/unit/test_navigation.py
@@ +3,4 @@
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
>  from marionette_test import MarionetteTestCase
> +from marionette import MarionetteException, TimeoutException

Please put this back to multiple lines

@@ +14,5 @@
> +        self.iframe_doc = self.marionette.absolute_url("test_iframe.html")
> +
> +    def test_set_location_through_execute_script(self):
> +        self.marionette.execute_script(
> +            "window.location.href = '%s'" % self.test_doc)

nit: if we are going to be multilining here it would be better to split around the %. It's better to align with the delimiter if possible

@@ +23,4 @@
>      def test_navigate(self):
> +        self.marionette.navigate(self.test_doc)
> +        self.assertNotEqual("about:blank", self.marionette.execute_script(
> +            "return window.location.href;"))

Nit: as above

also <3 it for looking neater

@@ +73,5 @@
> +        self.assertTrue(self.marionette.execute_script("""
> +        var elem = window.document.createElement('div');
> +        elem.id = 'someDiv';
> +        window.document.body.appendChild(elem);
> +        return true;"""))

If possible can we indent here so it doesnt line up with the python

	Andreas Tolfsen ❲:ato❳ Assignee
	Comment 3 • 9 years ago

/r/6989 - Bug 984921: Clean up Marionette navigation tests

Pull down this commit:

hg pull -r 1844c906cc436ef378f07342051a44689fedba58 https://reviewboard-hg.mozilla.org/gecko/

	Andreas Tolfsen ❲:ato❳ Assignee
	Comment 4 • 9 years ago

Comment on attachment 8591248 [details]
MozReview Request: bz://984921/ato

/r/6989 - Bug 984921: Clean up Marionette navigation tests

Pull down this commit:

hg pull -r 1844c906cc436ef378f07342051a44689fedba58 https://reviewboard-hg.mozilla.org/gecko/

	Andreas Tolfsen ❲:ato❳ Assignee
	Comment 5 • 9 years ago

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d7dd44c4d895

	David Burns :automatedtester
	Comment 6 • 9 years ago

Comment on attachment 8591248 [details]
MozReview Request: bz://984921/ato

https://reviewboard.mozilla.org/r/6987/#review5781

Ship It!

	Andreas Tolfsen ❲:ato❳ Assignee
	Comment 7 • 9 years ago

Pushed to inbound:  https://hg.mozilla.org/integration/mozilla-inbound/rev/d286935cb11b

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 8 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/d286935cb11b