Build PASSED

Started: 2018-04-04 09:23:25
Finished: 2018-04-04 09:57:00

View more information about this build on:

• WPT PR Status
• TravisCI

@pozdnyakov, thank you for bring this to advance test automation for Generic Sensor.
One concern, why we don't use WebDriver based automation solution? As it is more popular in WPT.

And if we use Test API, we should definitely defined a Test API spec for both test contributor and future implementations. Just like what WebUSB had done at https://wicg.github.io/webusb/test/

@Honry thanks for your comments! Could we proceed the review process in https://chromium-review.googlesource.com/c/chromium/src/+/980886, so that we have all the comments consolidated in a single place.

@pozdnyakov, thank you for bring this to advance test automation for Generic Sensor.
One concern, why we don't use WebDriver based automation solution? As it is more popular in WPT.

For the Generic Sensor implementation in Chromium we want to test the whole bindings code in the dedicated module, this could be achieved only with mocking the involved mojo interfaces. Not sure WebDriver can do this.

And if we use Test API, we should definitely defined a Test API spec for both test contributor and future implementations. Just like what WebUSB had done at https://wicg.github.io/webusb/test/

So far it's only initialize and reset methods.. Unless the test API becomes more complex, I'd not create a new specification for it.

@Honry thanks for your comments! Could we proceed the review process in https://chromium-review.googlesource.com/c/chromium/src/+/980886, so that we have all the comments consolidated in a single place.

@pozdnyakov, sorry for the inconvenient, I will review in chromium for your next patch.

So far it's only initialize and reset methods.. Unless the test API becomes more complex, I'd not create a new specification for it.

So, will you bring more methods to WPT?

@pozdnyakov, Travis is failing because when running these tests against Chrome Dev, the results were unstable. From https://travis-ci.org/w3c/web-platform-tests/jobs/366060782, can you tell why that is the case instead of them all failing consistently due to the lack of mojo mocking? We shouldn't change this PR unless we really have to, if there's some fix needed then a separate Chromium CL that we export roughly at the same time is better.

@jgraham FYI that this introduces something that's special for Chrome and will only work with extra command line arguments, the same as for Bluetooth and USB: --enable-blink-features=MojoJS,MojoJSTest

Because this doesn't expose any new API surface but just makes it seem like sensors exist it's not very hard to see a WebDriver path forward for this, and #9686 is the tracking issue.

I ran these tests locally, got the same error, MojoInterfaceInterceptor is not defined.

Error appears upon starting the browser that “you are using an unsupported command line flag: --enable-blink-features=MojoJS,MojoTest. Stability and security will suffer”

@Honry how about if you pass the flags as documented in http://web-platform-tests.org/running-tests/chrome.html?

Yeah, it works, but another consistent error: ReferenceError: testRunner is not defined,
root cause: the testRunner at line 204 from resources/chromium/generic_sensor_mocks.js is undefined.

Moreover, @pozdnyakov, tests under following folders should be updated as well, because they are using the same framework: generic-sensor-tests.js, I can do this once this PR is merged.

• ambient-light
• geolocation-sensor
• proximity

Ah, if (testRunner) needs to be if (window.testRunner). Can you see if that fixes the problem? If so can you make the change in Chromium? Then I'll merge this and the other change will be exported automatically.

@foolip, yes, window.testRunner can fix the problem, actually testRunner.setPermission only works in Chromium testRunner, we can skip it in WPT testing since at present the permission of sensors is enabled via "GenericSensor" and "GenericSensorExtraClasses" flags.

But there coming with another consistent error :-/
Mock sensor initialization fails at following code snippet,
generic-sensor-mocks.js

124 +        this.binding_.setConnectionErrorHandler(() => {
125 +          console.error("Mojo connection error");
126 +          this.reset();
127 +        });

While which can be pass in Chromium Layout testing (test path: LayoutTests/external/wpt/), @pozdnyakov, could you pls. take a look?

That's probably because of web-platform-tests/results-collection#81, right?

No, I've already passed the Mojo flags, --enable-blink-features=MojoJS,MojoJSTest, and tested WebUSB tests which can succeed to create an mock USB.

Oh. Well then I have no idea, hopefully @pozdnyakov does :)

Looking.. Sorry for the delay

@pozdnyakov any luck? Should we merge this and look into the fix separately, or what's the most effective way to resolve this?

Just to see if the flakiness has gone away, as in #10497, I'll rebase this to trigger another Travis run.

cc @alexshalamov

restarts travis

@foolip sorry for the late notice. I'm not working on the project starting from this month. Unfortunately I was not able to fix the flakiness before I left. I think someone from my team can proceed with this. @alexshalamov could you PTAL?

@alexshalamov will you be able to look at this? Note that after about a month the Chromium-side CL will fall outside of the window of considered commits and the change will be implicitly reverted by import. And... it turns out this happened 5 days ago:
https://chromium.googlesource.com/chromium/src/+/9bb566abdfddab2e5acda5e1525be9c8e343a7cc

That's really unfortunate, because these were good changes. I think the cleanest thing to recover is to just merge this now. @Hexcles, do you know how we could notice when this is approaching to raise the urgency of a blocked export when it gets close?

I've rebased the branch (chromium-export-cl-980886) on master. When Travis has finished running, if the only Travis failures are in the Chrome stability job, I will merge this PR and file a bug to track the flakiness.

Merged with this failure:
https://travis-ci.org/w3c/web-platform-tests/jobs/376549386

It's possible and even likely that some of these tests will now be flaky in Chrome Dev, so I'll file another issue for that.

@foolip thanks for putting out the fire.

The window used to be around a month, and only few un-merged exports have been overwritten by imports so far. Yet apparently the window can bu much shorter in reality (see https://crbug.com/752214), and when overwriting does happen, things can be messy and confusing. I'll increase the history window for now to give us a longer window to handle unmerged exports for now, and think about a long-term solution for this "ultimate" failure mode of the exporter.