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

Run a couple WPT tests on Android on CI #21242

Merged
merged 2 commits into from Jul 28, 2018
Merged

Run a couple WPT tests on Android on CI #21242

merged 2 commits into from Jul 28, 2018

Conversation

@SimonSapin
Copy link
Member

SimonSapin commented Jul 24, 2018

This change is Reviewable

@highfive
Copy link

highfive commented Jul 24, 2018

Heads up! This PR modifies the following files:

@SimonSapin
Copy link
Member Author

SimonSapin commented Jul 24, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Jul 24, 2018

Trying commit fac9acc with merge 7c4cce3...

bors-servo added a commit that referenced this pull request Jul 24, 2018
Run a couple WPT tests on Android on CI
@bors-servo
Copy link
Contributor

bors-servo commented Jul 24, 2018

💔 Test failed - android-x86

@highfive
Copy link

highfive commented Jul 24, 2018

  ▶ TIMEOUT [expected OK] /_mozilla/mozilla/DOMParser.html

  ▶ TIMEOUT [expected OK] /_mozilla/mozilla/webgl/context_creation_error.html
@SimonSapin
Copy link
Member Author

SimonSapin commented Jul 24, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Jul 24, 2018

Trying commit c99c93f with merge 90e1a38...

bors-servo added a commit that referenced this pull request Jul 24, 2018
Run a couple WPT tests on Android on CI

<!-- 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/21242)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 24, 2018

💔 Test failed - android-x86

@SimonSapin
Copy link
Member Author

SimonSapin commented Jul 24, 2018

 0:37.72 TEST_START: /_mozilla/mozilla/DOMParser.html
 0:39.23 pid:4903 Entering android_main
 0:39.23 pid:4903 Creating application thread
 0:39.23 pid:4903 ERROR 2018-07-24T21:32:43Z: script::dom::bindings::error: Error at :2:1 timeout is not defined
 0:53.27 TEST_END: TIMEOUT, expected OK

So far I did not reproduce this failure on my linux machine. I think that the undefined JS variable is the first argument in this call:

window.__wd_results_timer__ = setTimeout(timeout, %(timeout)s);

… and that it is supposed to be defined here:

function timeout() {
if (tests.timeout_length === null) {
tests.timeout();
}
}
expose(timeout, 'timeout');

… with expose() effectively doing global_scope["timeout"] = timeout.

The former script is injected through WebDriver here:

result = json.loads(
session.execute_async_script(
self.script % {"abs_url": url,
"url": strip_server(url),
"timeout_multiplier": self.timeout_multiplier,
"timeout": timeout * 1000}))

The latter is part of every test like this:

<script src="/resources/testharness.js"></script>

@SimonSapin
Copy link
Member Author

SimonSapin commented Jul 25, 2018

Although this issue does not reproduce on my linux desktop (mach test-wpt-android /_mozilla/mozilla/DOMParser.html --release succeeds), it does reproduce on my linux laptop and on my macbook. Maybe it is timing-related (the desktop has a faster CPU), but adding time.sleep(1) before the execute_async_script(…) call does not help.

@jdm I’m running out of leads here, I don’t know what to try next.

@jdm
Copy link
Member

jdm commented Jul 25, 2018

The code you linked seems like a reasonable lead. What about debug output to show what code is actually executing?

@SimonSapin
Copy link
Member Author

SimonSapin commented Jul 25, 2018

Mystery solved.

I was assuming that there would be no issue with the test loading /resources/testharness.js, but sprinkling some console.log(something) lines here and there showed that this is not happening at all on machines where I observe the issue. The test is not loading at all!

wptrunner asks Servo (in this case through WebDriver) to load URLs like http://web-platform.test:8000/_mozilla/mozilla/DOMParser.html. It also generates a temporary file with a syntax compatible with /etc/hosts that maps the hostname web-platform.test and some others to 127.0.01, and sets the HOST_FILE environment variable to tell Servo where to find it. However in the case of the Android emulator, the host file does not exist in the environment’s filesystem and environment variables are not propagated. So Servo gets a DNS error while trying to resolve the hostname web-platform.test and the document load fails. Additionally, because of #21248, that failure is not propagated back to the WebDriver client. The WebDriver command appears to succeed, so wptrunner carries on and only later times out when test results do not arrive.

As to what happens on my Linux desktop machine, DNS resolution in the Android emulator appears to be delegated to the host. And web-platform.test is valid on this machine because I had modified /etc/hosts by hand weeks ago when wptserve asked me to do so when I tried to use it without mach.

So what’s needed is, most likely:

  • Adding a mechanism similar to /sdcard/Android/data/com.mozilla.servo/files/android_params to allow setting environment variables in the Servo process on Android. (With code at startup that reads a file and calls std::env::set_var.)
  • Add support in etc/run_in_headless_android_emulator.py for the HOST_FILE env variable, similar to existing support for --user-stylesheet: use adb push to make the file available in the emulator’s filesystem, and then use new mechanism to propagate the env variable
@SimonSapin SimonSapin force-pushed the android-wpt branch from c99c93f to 67b47b0 Jul 26, 2018
@SimonSapin SimonSapin force-pushed the android-wpt branch from 67b47b0 to d3107a3 Jul 26, 2018
@SimonSapin
Copy link
Member Author

SimonSapin commented Jul 26, 2018

I ended up adding not a general mechanism for env variables, but a special case for /sdcard/Android/data/com.mozilla.servo/files/android_hosts.

I’ve tested this through SSH on servo-linux-kvm1, this should be ready to land.

@nox
Copy link
Member

nox commented Jul 26, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Jul 26, 2018

📌 Commit d3107a3 has been approved by nox

@highfive highfive assigned nox and unassigned emilio Jul 26, 2018
@jdm
Copy link
Member

jdm commented Jul 28, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2018

Testing commit 5e5b89a with merge b94fd23...

bors-servo added a commit that referenced this pull request Jul 28, 2018
Run a couple WPT tests on Android on CI

<!-- 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/21242)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2018

💔 Test failed - linux-rel-wpt

@jdm
Copy link
Member

jdm commented Jul 28, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2018

💔 Test failed - linux-rel-wpt

@jdm
Copy link
Member

jdm commented Jul 28, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2018

Testing commit 5e5b89a with merge e9bc6f4...

bors-servo added a commit that referenced this pull request Jul 28, 2018
Run a couple WPT tests on Android on CI

<!-- 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/21242)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2018

💔 Test failed - mac-rel-css2

@jdm
Copy link
Member

jdm commented Jul 28, 2018

@bors-servo retry

  • Websockets
@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2018

@bors-servo bors-servo merged commit 5e5b89a into master Jul 28, 2018
3 of 5 checks passed
3 of 5 checks passed
Tidelift An error occured
Details
Taskcluster (pull_request) TaskGroup: failure
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@SimonSapin SimonSapin deleted the android-wpt branch Aug 1, 2018
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.