Remove deprecated and unused capabilities from Marionette

RESOLVED FIXED in Firefox 52

Status

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: whimboo, Assigned: ato)

Tracking

Version 3
mozilla53
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox52 fixed, firefox53 fixed)

Details

(URL)

MozReview Requests

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

Attachments

(6 attachments)

(Reporter)

Description

2 years ago
Andreas, maybe you accidentally introduced this by bug 1217988? I don't see a reason why we have to transfer the version property twice.
Flags: needinfo?(ato)
(Assignee)

Comment 1

2 years ago
The browserVersion is the mandated WebDriver capability.  `version` is scheduled for removal, but we need to update any code using it first.
Flags: needinfo?(ato)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Assignee: nobody → ato
Status: NEW → ASSIGNED
(Assignee)

Updated

2 years ago
Summary: sessionCapabilities send application version twice as browserVersion and version → Remove deprecated and unused capabilities from Marionette
(Reporter)

Comment 8

2 years ago
mozreview-review
Comment on attachment 8810652 [details]
Bug 1317462 - Add test for processId capability;

https://reviewboard.mozilla.org/r/92938/#review92968

::: testing/marionette/harness/marionette/runner/mixins/reporting.py:143
(Diff revision 1)
>          date_format = '%d %b %Y %H:%M:%S'
>          version = {}
>  
>          if self.capabilities:
> +            with self.using_context("chrome"):
> +                appinfo = self.execute_script("return Service.appinfo")

This file is gone soon via bug 1316707
(Reporter)

Comment 9

2 years ago
mozreview-review
Comment on attachment 8810653 [details]
Bug 1317462 - Remove appBuildId capability from Marionette;

https://reviewboard.mozilla.org/r/92940/#review92972

::: testing/marionette/harness/marionette/tests/unit/test_capabilities.py
(Diff revision 1)
>          self.assertEqual(self.caps["platform"], self.caps["platformName"].upper())
>  
> -    def test_extensions(self):
> -        self.assertIn("appBuildId", self.caps)
> -
> -        self.assertEqual(self.caps["appBuildId"], self.appinfo["appBuildID"])

We should leave this method in to test for the processId.
(Assignee)

Comment 10

2 years ago
mozreview-review-reply
Comment on attachment 8810653 [details]
Bug 1317462 - Remove appBuildId capability from Marionette;

https://reviewboard.mozilla.org/r/92940/#review92972

> We should leave this method in to test for the processId.

Fixed in new patch.
(Assignee)

Comment 11

2 years ago
mozreview-review-reply
Comment on attachment 8810652 [details]
Bug 1317462 - Add test for processId capability;

https://reviewboard.mozilla.org/r/92938/#review92968

> This file is gone soon via bug 1316707

Thanks for the head’s up, but I don’t believe this is an issue.  Will be resolved during conflict resolution at rebase.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 33

2 years ago
mozreview-review
Comment on attachment 8810655 [details]
Bug 1317462 - Remove screenshot capabilities from Marionette;

https://reviewboard.mozilla.org/r/92944/#review93852
Attachment #8810655 - Flags: review?(dburns) → review+

Comment 34

2 years ago
mozreview-review
Comment on attachment 8810653 [details]
Bug 1317462 - Remove appBuildId capability from Marionette;

https://reviewboard.mozilla.org/r/92940/#review93854
Attachment #8810653 - Flags: review?(dburns) → review+

Comment 35

2 years ago
mozreview-review
Comment on attachment 8810651 [details]
Bug 1317462 - Remove XULappId capability from Marionette;

https://reviewboard.mozilla.org/r/92936/#review93856
Attachment #8810651 - Flags: review?(dburns) → review+

Comment 36

2 years ago
mozreview-review
Comment on attachment 8810652 [details]
Bug 1317462 - Add test for processId capability;

https://reviewboard.mozilla.org/r/92938/#review93858
Attachment #8810652 - Flags: review?(dburns) → review+

Comment 37

2 years ago
mozreview-review
Comment on attachment 8810650 [details]
Bug 1317462 - Remove version capability from Marionette;

https://reviewboard.mozilla.org/r/92934/#review94036
Attachment #8810650 - Flags: review?(dburns) → review+

Comment 38

2 years ago
mozreview-review
Comment on attachment 8810654 [details]
Bug 1317462 - Remove platform capability from Marionette;

https://reviewboard.mozilla.org/r/92942/#review94038
Attachment #8810654 - Flags: review?(dburns) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 45

2 years ago
Pushed by atolfsen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c46f4cea2b2d
Remove version capability from Marionette; r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/317523c7dd69
Remove XULappId capability from Marionette; r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/4cd9a4f42f35
Remove appBuildId capability from Marionette; r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/6c00a7b80b3c
Remove platform capability from Marionette; r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/e718e037a6c5
Remove screenshot capabilities from Marionette; r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/0e1745003c29
Add test for processId capability; r=automatedtester
(Reporter)

Updated

2 years ago
Depends on: 1319692
(Assignee)

Comment 47

2 years ago
Sheriffs: Uplift please, as dependency to uplifting bug 1103196 (will upload patches soon).  Test-only, as Marionette is behind flag.
Whiteboard: [checkin-needed-aurora]
(Reporter)

Comment 49

2 years ago
(In reply to Andreas Tolfsen ‹:ato› from comment #47)
> Sheriffs: Uplift please, as dependency to uplifting bug 1103196 (will upload
> patches soon).  Test-only, as Marionette is behind flag.

When you request uplifts please also make sure that known regressions also gets uplifted. This didn't happen in this case. Thanks. So I will do so now on bug 1319692.
You need to log in before you can comment on or make changes to this bug.