Closed Bug 1368264 Opened 7 years ago Closed 7 years ago

Hook testing/geckodriver up to WPT

Categories

(Remote Protocol :: Marionette, enhancement, P1)

Version 3
enhancement

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: nobody → ato
Status: NEW → ASSIGNED
Depends on: 1368035
Priority: -- → P1
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
Attachment #8873557 - Flags: review?(nfroyd)
Attachment #8873556 - Flags: review?(james)
Attachment #8873558 - Flags: review?(james)
Attachment #8874389 - Flags: review?(james)
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 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 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+
Attachment #8873556 - Flags: review?(james) → 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+
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.
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.
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.
Blocks: 1370636
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 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.
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
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: