Closed Bug 1180818 Opened 5 years ago Closed 5 years ago

Improve Autoland UI testing

Categories

(MozReview Graveyard :: General, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: dminor, Assigned: dminor)

References

Details

Attachments

(7 files)

+++ 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.
Assignee: nobody → dminor
Priority: -- → P1
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.
Attachment #8641448 - Flags: review?(mdoglio)
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.
Attachment #8641449 - Flags: review?(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.
Attachment #8641450 - Flags: review?(gps)
testing: set autoland user in hgrc when cloning repositories (bug 1180818) r?gps
Attachment #8641451 - Flags: review?(gps)
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.
Attachment #8641453 - Flags: review?(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.
Attachment #8641454 - Flags: review?(mdoglio)
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 :/
Attachment #8641449 - Flags: review?(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!
Attachment #8641450 - Flags: review?(gps) → review+
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!
Attachment #8641451 - Flags: review?(gps) → review+
Comment on attachment 8641452 [details]
MozReview Request: testing: add autoland@example.com to scm_level_* groups (bug 1180818) r?gps

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

Ship It!
Attachment #8641452 - Flags: review?(gps) → review+
Attachment #8641453 - Flags: review?(mdoglio) → review+
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 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!
Attachment #8641448 - Flags: review?(mdoglio) → review+
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
Attachment #8641454 - Flags: review?(mdoglio) → review+
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.
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.
Product: Developer Services → MozReview
You need to log in before you can comment on or make changes to this bug.