1180818 - Improve Autoland UI testing

Status: RESOLVED FIXED

(MozReview Graveyard :: General, defect, P1)

Description

[Dan Minor [:dminor]]

+++ This bug was initially created as a clone of Bug #1174752 +++

Most of the Autoland tests were written prior to having reviewboard and mercurial in docker containers. We're now in a position to be able to write much better integration tests and we should do so before we start landing revisions to an inbound tree.

I'm splitting this off of Bug 1174752, that bug will focus on .t style tests for Autoland, this one will be for Selenium tests.

Comment 1

[Dan Minor [:dminor]]

Attachment: MozReview Request: testing: remove old autoland UI tests (bug 1180818) r?mdoglio

testing: remove old autoland UI tests (bug 1180818) r?mdoglio

Now that reviewboard and autoland are running in docker, we can replace these
partial tests with full integration tests that have round trip communication
to autoland.

Comment 2

[Dan Minor [:dminor]]

Attachment: MozReview Request: testing: disable caching for selenium tests (bug 1180818) r?gps

testing: disable caching for selenium tests (bug 1180818) r?gps

We see stale content when running the tests. It would be great to fix the
underlying cause, but for now, let's disable caching in the browser so we
can get the tests running properly.

Comment 3

[Dan Minor [:dminor]]

Attachment: MozReview Request: testing: move assign-reviewer to base class (bug 1180818) r?gps

testing: move assign-reviewer to base class (bug 1180818) r?gps

This moves the assign reviewer code from test-reviewer-selection to the base
class so it can be used from other tests.

This also adds some utility methods for dumping the autoland and reviewboard
logs to the base class which are useful for debugging failing tests.

Comment 4

[Dan Minor [:dminor]]

Attachment: MozReview Request: testing: set autoland user in hgrc when cloning repositories (bug 1180818) r?gps

testing: set autoland user in hgrc when cloning repositories (bug 1180818) r?gps

Comment 5

[Dan Minor [:dminor]]

Attachment: MozReview Request: testing: add [e-mail] to scm_level_* groups (bug 1180818) r?gps

testing: add [e-mail] to scm_level_* groups (bug 1180818) r?gps

Comment 6

[Dan Minor [:dminor]]

Attachment: MozReview Request: autoland: ensure repository directory exists (bug 1180818) r?mdoglio

autoland: ensure repository directory exists (bug 1180818) r?mdoglio

I noticed this while fixing up the Autoland selenium tests. If the directory
path does not exist, subprocess will throw an uncaught OSError after the new
bugzilla bug has been filed, causing new bugs to be filed several times a
second when Autoland retries the failed request. This isn't something that's
likely to happen in production, but better safe than sorry.

Comment 7

[Dan Minor [:dminor]]

Attachment: MozReview Request: testing: fixup test-autoland-try and test-import-pullrequest (bug 1180818) r?mdoglio

testing: fixup test-autoland-try and test-import-pullrequest (bug 1180818) r?mdoglio

This modifies the autoland tests to work with successful autoland requests.
Previously due to deficiencies in our testing infrastructure, the requests
would always fail.

Comment 8

[Gregory Szorc [:gps]]

Comment on attachment 8641449 [details]
MozReview Request: testing: disable caching for selenium tests (bug 1180818) r?gps

https://reviewboard.mozilla.org/r/14533/#review13117

This feels somewhat wrong to me. If the browser cache isn't working properly then ReviewBoard is functioning incorrectly wrt HTTP cache behavior, no? That, or the Selenium tests aren't reflective of what a user does when they use the browser. Either way, something is wrong. This feels like sweeping dust under the rug :/

Comment 9

[Gregory Szorc [:gps]]

Comment on attachment 8641450 [details]
MozReview Request: testing: move assign-reviewer to base class (bug 1180818) r?gps

https://reviewboard.mozilla.org/r/14535/#review13119

Ship It!

Comment 10

[Gregory Szorc [:gps]]

Comment on attachment 8641451 [details]
MozReview Request: testing: set autoland user in hgrc when cloning repositories (bug 1180818) r?gps

https://reviewboard.mozilla.org/r/14537/#review13121

Ship It!

Comment 11

[Gregory Szorc [:gps]]

Comment on attachment 8641452 [details]
MozReview Request: testing: add [e-mail] to scm_level_* groups (bug 1180818) r?gps

https://reviewboard.mozilla.org/r/14539/#review13123

Ship It!

Comment 12

[Mauro Doglio [:mdoglio]]

Comment on attachment 8641453 [details]
MozReview Request: autoland: ensure repository directory exists (bug 1180818) r?mdoglio

https://reviewboard.mozilla.org/r/14541/#review13385

Ship It!

Comment 13

[Mauro Doglio [:mdoglio]]

Comment on attachment 8641448 [details]
MozReview Request: testing: remove old autoland UI tests (bug 1180818) r?mdoglio

https://reviewboard.mozilla.org/r/14531/#review13387

Ship It!

Comment 14

[Mauro Doglio [:mdoglio]]

Comment on attachment 8641454 [details]
MozReview Request: testing: fixup test-autoland-try and test-import-pullrequest (bug 1180818) r?mdoglio

https://reviewboard.mozilla.org/r/14543/#review13389

::: pylib/mozreview/mozreview/tests/test-import-pullrequest.py:29
(Diff revision 1)
> +            

remove blank spaces

::: pylib/mozreview/mozreview/tests/test-import-pullrequest.py:75
(Diff revision 1)
> -        self.assertTrue('Importing pull request' in element.text)
> +            self.bugzilla().client.get(2)

We could also/instead check that the rb ui shows the correct bug number in the right column

::: pylib/mozreview/mozreview/tests/test-import-pullrequest.py:51
(Diff revision 1)
> +            # we check the text, but we want to be able to catch the error 

remove trailing space

::: pylib/mozreview/mozreview/tests/test-import-pullrequest.py:73
(Diff revision 1)
> -        # This should be present regardless of whether or not the request has
> +        # Autoland should reuse the existing bug for this pull request. 

remove trailing space

Comment 15

[Dan Minor [:dminor]]

https://reviewboard.mozilla.org/r/14533/#review13117

Well, I filed Bug 1188979 to get this stuff fixed properly. It doesn't feel like going down the rabbit hole of whatever is broken with our caching (which I've noticed outside of these tests) should be a prerequisite of getting these changes landed. But I'm willing to land the tests broken with a comment explaining why if that is preferable.

Comment 16

[Dan Minor [:dminor]]

https://reviewboard.mozilla.org/r/14543/#review13389

> We could also/instead check that the rb ui shows the correct bug number in the right column

Good idea, I'd like to both just in case Autoland accidentally creates a new bug but then uses the old one anyway.

Comment 17

[Dan Minor [:dminor]]

Thanks, pushed to:

remote:   https://hg.mozilla.org/hgcustom/version-control-tools/rev/edff6d9fd7f6
remote:   https://hg.mozilla.org/hgcustom/version-control-tools/rev/39a06c92a473
remote:   https://hg.mozilla.org/hgcustom/version-control-tools/rev/35b96b6321c6
remote:   https://hg.mozilla.org/hgcustom/version-control-tools/rev/86b9275db32a
remote:   https://hg.mozilla.org/hgcustom/version-control-tools/rev/0482431406ba
remote:   https://hg.mozilla.org/hgcustom/version-control-tools/rev/750e44614434