Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start running webdriver tests #13261

Closed
wants to merge 9 commits into from
Closed

Start running webdriver tests #13261

wants to merge 9 commits into from

Conversation

@jdm
Copy link
Member

jdm commented Sep 13, 2016

This branch includes the patches necessary to start running webdriver tests consistently.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • There are tests for these changes

This change is Reviewable

@highfive
Copy link

highfive commented Sep 13, 2016

Heads up! This PR modifies the following files:

  • @asajeffrey: components/webdriver_server/lib.rs
  • @wafflespeanut: python/servo/devenv_commands.py
  • @jgraham: tests/wpt/harness/wptrunner/wpttest.py, tests/wpt/harness/wptrunner/wptmanifest/tests/test_serializer.py, tests/wpt/include.ini, tests/wpt/harness/wptrunner/browsers/servo.py, tests/wpt/harness/wptrunner/webdriver_server.py, tests/wpt/harness/wptrunner/wptcommandline.py, tests/wpt/harness/wptrunner/executors/executormarionette.py, tests/wpt/harness/wptrunner/tests/test_chunker.py, tests/wpt/harness/wptrunner/executors/pytestrunner/fixtures.py, tests/wpt/harness/wptrunner/executors/executorservo.py, tests/wpt/harness/wptrunner/wptrunner.py, tests/wpt/harness/wptrunner/browsers/firefox.py, tests/wpt/harness/wptrunner/browsers/init.py, tests/wpt/harness/wptrunner/tests/test_update.py, tests/wpt/harness/wptrunner/executors/executorservodriver.py, tests/wpt/harness/wptrunner/executors/webdriver.py, tests/wpt/harness/wptrunner/executors/executorselenium.py, components/webdriver_server/lib.rs, tests/wpt/harness/wptrunner/executors/testharness_webdriver.js, tests/wpt/harness/wptrunner/testrunner.py, tests/wpt/harness/wptrunner/tests/test_hosts.py
@jdm
Copy link
Member Author

jdm commented Sep 13, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Sep 13, 2016

Trying commit 739f0d2 with merge 7e55e86...

bors-servo added a commit that referenced this pull request Sep 13, 2016
Start running webdriver tests

This branch includes the patches necessary to start running webdriver tests consistently.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] There are tests for these changes
@jdm
Copy link
Member Author

jdm commented Sep 13, 2016

3c04711, c73ecf7, cdfe796, and 2449bc7 will need to be upstreamed.

@bors-servo
Copy link
Contributor

bors-servo commented Sep 14, 2016

💔 Test failed - linux-rel

@jdm
Copy link
Member Author

jdm commented Sep 14, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Sep 14, 2016

Trying commit cdfe796 with merge c06d7f2...

bors-servo added a commit that referenced this pull request Sep 14, 2016
Start running webdriver tests

This branch includes the patches necessary to start running webdriver tests consistently.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13261)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Sep 14, 2016

💔 Test failed - linux-rel

@jdm
Copy link
Member Author

jdm commented Sep 14, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Sep 14, 2016

Trying commit 2449bc7 with merge f653ef8...

bors-servo added a commit that referenced this pull request Sep 14, 2016
Start running webdriver tests

This branch includes the patches necessary to start running webdriver tests consistently.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13261)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Sep 14, 2016

💔 Test failed - linux-rel

@jdm
Copy link
Member Author

jdm commented Sep 14, 2016

I fear that the webdriver tests don't deal well with concurrent processes. I'm not sure how to reduce the number of processes used only for that test type, however. Any ideas, @jgraham?

@jgraham
Copy link
Contributor

jgraham commented Sep 14, 2016

Run them using a separate command on buildbot which limits the test type using --test-type wdspec and process count using --processes=1? The existing invocation will also need to select the test types it wants to run rather than doing everything.

@jdm
Copy link
Member Author

jdm commented Sep 15, 2016

Bah, that's impossibledifficult to test until we finish converting our buildbot instances to use in-tree test configurations :(

@jdm
Copy link
Member Author

jdm commented Sep 15, 2016

Specifically, blocked on servo/saltfs#316.

@bors-servo
Copy link
Contributor

bors-servo commented Oct 18, 2016

The latest upstream changes (presumably #13614) made this pull request unmergeable. Please resolve the merge conflicts.

@aneeshusa
Copy link
Member

aneeshusa commented Oct 19, 2016

The final PR for servo/saltfs#316 has been merged and deployed, so this is no longer blocked.

@cbrewster
Copy link
Member

cbrewster commented Oct 29, 2016

r? @jgraham or someone else

@highfive highfive assigned jgraham and unassigned cbrewster Oct 29, 2016
@jgraham
Copy link
Contributor

jgraham commented Nov 2, 2016

Remember to upstream changes to wptrunner.


Reviewed 1 of 1 files at r1, 1 of 1 files at r2, 16 of 18 files at r3, 1 of 1 files at r5, 1 of 1 files at r6, 1 of 1 files at r7, 1 of 1 files at r8, 1 of 1 files at r9.
Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.


components/webdriver_server/lib.rs, line 509 at r9 (raw file):

    fn handle_dismiss_alert(&mut self) -> WebDriverResult<WebDriverResponse> {
        //TODO figure out how to interact with blocking native UI
        Ok(WebDriverResponse::Void)

Can't we return unsupported operation for now?


tests/wpt/harness/wptrunner/executors/executorservo.py, line 295 at r9 (raw file):

    def setup(self, runner):
        try:
            self.server = ServoDriverServer(self.logger, binary=self.browser.binary, binary_args=self.browser.binary_args, render_backend=self.browser.render_backend)

Some more linebreaks here would be nice.


tests/wpt/harness/wptrunner/executors/executorservo.py, line 334 at r9 (raw file):

class ServoWdspecExecutor(WdspecExecutor):

Not necessarily a blocker, but this seems to share all of the code with MarionetteWdSpecExecutor except the protocol. Maybe we can construct a base class that is just parameterized by the protocol class.


Comments from Reviewable

@jdm
Copy link
Member Author

jdm commented Jan 27, 2017

Tracking this work in #15274.

@jdm jdm closed this Jan 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.