Closed
Bug 1368264
Opened 7 years ago
Closed 7 years ago
Hook testing/geckodriver up to WPT
Categories
(Remote Protocol :: Marionette, enhancement, P1)
Tracking
(firefox55 fixed)
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: ato, Assigned: ato)
References
Details
Attachments
(4 files)
When ENABLE_GECKODRIVER (introduced in https://bugzilla.mozilla.org/show_bug.cgi?id=1368035), we should copy the geckodriver binary to dist/bin so it can be included with the test package we ship to test slaves.
Assignee | ||
Updated•7 years ago
|
Assignee | ||
Comment 1•7 years ago
|
||
We also want to hook the geckodriver binary produces on the build slaves to the WPT wdspec tests.
Summary: Copy geckodriver binary to dist/bin → Hook testing/geckodriver up to WPT
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) |
Assignee | ||
Updated•7 years ago
|
Attachment #8873557 -
Flags: review?(nfroyd)
Attachment #8873556 -
Flags: review?(james)
Attachment #8873558 -
Flags: review?(james)
Attachment #8874389 -
Flags: review?(james)
Assignee | ||
Comment 11•7 years ago
|
||
Latest try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cfbbe0d6f32d
The Wr job on Win x64 [sic] has since been corrected on m-c, but this branch doesn’t have that commit. The intermittents on Wd for Linux x64 Debug are known.
Comment 12•7 years ago
|
||
mozreview-review |
Comment on attachment 8873557 [details]
Bug 1368264 - Fix missing RUST_PROGRAMS entry in if-condition
https://reviewboard.mozilla.org/r/144942/#review150188
Is geckodriver really a target program? Is that really correct for Android?
Attachment #8873557 -
Flags: review?(nfroyd) → review+
Comment 13•7 years ago
|
||
mozreview-review |
Comment on attachment 8874389 [details]
Bug 1368264 - Make WPT use WebDriver binary from test archive
https://reviewboard.mozilla.org/r/145752/#review150194
::: testing/mozharness/scripts/web_platform_tests.py:177
(Diff revision 3)
> if "wdspec" in c.get("test_type", []):
> - assert self.geckodriver_path is not None
> - cmd.append("--webdriver-binary=%s" % self.geckodriver_path)
> + geckodriver_path = os.path.join(dirs["abs_test_bin_dir"], "geckodriver")
> + if not os.path.isfile(geckodriver_path):
> + self.fatal("Unable to find geckodriver binary "
> + "in common test package: %s" % geckodriver_path)
> + cmd.append("--webdriver-binary=%s" % geckodriver_path)
I guess this will fail on Windows, but OK to fix that later.
Attachment #8874389 -
Flags: review?(james) → review+
Comment 14•7 years ago
|
||
mozreview-review |
Comment on attachment 8873556 [details]
Bug 1368264 - Have mach pick up geckodriver from dist/bin
https://reviewboard.mozilla.org/r/144940/#review150196
Attachment #8873556 -
Flags: review?(james) → review+
Comment 15•7 years ago
|
||
mozreview-review |
Comment on attachment 8873558 [details]
Bug 1368264 - Include geckodriver in test archive
https://reviewboard.mozilla.org/r/144944/#review150200
::: python/mozbuild/mozbuild/action/test_archive.py:40
(Diff revision 2)
> - 'SmokeDMD',
> 'certutil',
> 'crashinject',
> 'fileid',
> + 'geckodriver',
> + 'GenerateOCSPResponse',
Does this work? I expect it to complain that it's not in unicode codepoint order anymore. I suggest reverting these changes.
Attachment #8873558 -
Flags: review?(james) → review+
Assignee | ||
Comment 16•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8873558 [details]
Bug 1368264 - Include geckodriver in test archive
https://reviewboard.mozilla.org/r/144944/#review150200
> Does this work? I expect it to complain that it's not in unicode codepoint order anymore. I suggest reverting these changes.
I just ran sort(1) over the list to put geckodriver in the right place, which I guess reordered it in lexicographical order. I don’t see any regressions from this change, but if you think it’s important I can edit the patch again.
Assignee | ||
Comment 17•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8873557 [details]
Bug 1368264 - Fix missing RUST_PROGRAMS entry in if-condition
https://reviewboard.mozilla.org/r/144942/#review150188
It would be in the future. We have disabled compilation of geckodriver for Android at the moment because we need to figure out how to cross-compile it for the target system instead of the host. When this is fixed, it will also for this platform emit a geckodriver binary that we will want to move to dist/bin.
Assignee | ||
Comment 18•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8874389 [details]
Bug 1368264 - Make WPT use WebDriver binary from test archive
https://reviewboard.mozilla.org/r/145752/#review150194
> I guess this will fail on Windows, but OK to fix that later.
Good point. I’ve filed https://bugzilla.mozilla.org/show_bug.cgi?id=1370636 to enable the Wd job on Windows platforms. I will address this there.
Comment 19•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8873558 [details]
Bug 1368264 - Include geckodriver in test archive
https://reviewboard.mozilla.org/r/144944/#review150200
> I just ran sort(1) over the list to put geckodriver in the right place, which I guess reordered it in lexicographical order. I don’t see any regressions from this change, but if you think it’s important I can edit the patch again.
I'm moderately surprised the build jobs aren't failing, and I think that I would prefer this change is reversed; sorting by codepoint order is a legitimate sort method and in particular is the way Python sorts by default (which is therefore what I expect this to use).
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 24•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8873558 [details]
Bug 1368264 - Include geckodriver in test archive
https://reviewboard.mozilla.org/r/144944/#review150200
> I'm moderately surprised the build jobs aren't failing, and I think that I would prefer this change is reversed; sorting by codepoint order is a legitimate sort method and in particular is the way Python sorts by default (which is therefore what I expect this to use).
OK, thanks for the background.
Comment 25•7 years ago
|
||
Pushed by atolfsen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9eec39879b33
Fix missing RUST_PROGRAMS entry in if-condition r=froydnj
https://hg.mozilla.org/integration/autoland/rev/332dae5c381b
Have mach pick up geckodriver from dist/bin r=jgraham
https://hg.mozilla.org/integration/autoland/rev/3f7e348f0ff2
Include geckodriver in test archive r=jgraham
https://hg.mozilla.org/integration/autoland/rev/447882512ee8
Make WPT use WebDriver binary from test archive r=jgraham
Comment 26•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9eec39879b33
https://hg.mozilla.org/mozilla-central/rev/332dae5c381b
https://hg.mozilla.org/mozilla-central/rev/3f7e348f0ff2
https://hg.mozilla.org/mozilla-central/rev/447882512ee8
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Updated•2 years ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•