Allow MRO for Marionette testcase classes

RESOLVED FIXED in Firefox 52

Status

Testing
Marionette
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: whimboo, Assigned: whimboo)

Tracking

Version 3
mozilla52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr45 wontfix, firefox50 wontfix, firefox51 wontfix, firefox52 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
To be able to correctly use mixins with the Marionette testcase classes we have to prepare those classes for MRO (method resolution order). This is actually easy to do by changing the direct invocations of super class methods with super() like the following for the class TestSomething:

> MarionetteTestCase.setUp(self)

to

> super(TestSomething, self).setUp()

With that MRO will take care about the order of methods to be called. As of now we only call the method of the MarionetteTestCase class.

I need this feature for my work on bug 1259055 and it will also help a lot with bug 1310632 to decouple Firefox Puppeteer from the Firefox UI harness.

As for the latter I would like to get this patch uplifted down to Firefox 45.xESR to ensure we can do this flip as soon as possible.
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8802888 - Flags: review?(mjzffr)
(Assignee)

Updated

2 years ago
status-firefox50: --- → affected
status-firefox51: --- → affected
status-firefox52: --- → affected
status-firefox-esr45: --- → affected

Comment 2

2 years ago
mozreview-review
Comment on attachment 8802888 [details]
Bug 1311676 - Allow MRO for Marionette testcase classes.

https://reviewboard.mozilla.org/r/87162/#review86356

yay, super!
Attachment #8802888 - Flags: review?(mjzffr) → review+

Comment 3

2 years ago
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/04a3c1c6625c
Allow MRO for Marionette testcase classes. r=maja_zf

Comment 4

2 years ago
Landed 22 hours ago https://hg.mozilla.org/mozilla-central/rev/04a3c1c6625c
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox52: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
(Assignee)

Comment 5

2 years ago
test-only change which we would like to get uplifted. For now please do it for aurora and beta. I will request esr45 if beta is fine.
Whiteboard: [checkin-needed-aurora][checkin-needed-beta]
has problems uplifting to aurora:

warning: conflicts while merging testing/marionette/harness/marionette/marionette_test/testcases.py! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')
Flags: needinfo?(hskupin)
Whiteboard: [checkin-needed-aurora][checkin-needed-beta]
(Assignee)

Updated

2 years ago
Blocks: 1313312
(Assignee)

Comment 7

2 years ago
Basically the patch doesn't apply that well due to changes implemented with bug 1301153. 

While testing with MRO and Firefox puppeteer I noticed that all the changes we need are actually in the Marioentte client. Nothing in Marionette server is affected. As such and that our update tests are using the Marionette packages from the expected target build, I do not see a reason right now to work on updates on this patch to get uplifted.

If we have a need for an uplift I will fix the patch and re-request.
status-firefox50: affected → wontfix
status-firefox51: affected → wontfix
status-firefox-esr45: affected → wontfix
Flags: needinfo?(hskupin)
You need to log in before you can comment on or make changes to this bug.