Any feedback on this PR?

@louaybassbouss Quite a lot of us are OOO at the moment. 😔

Thanks @gsnedders for the info

Hi @louaybassbouss, sorry that this has gone so long without attention. Pinging @web-platform-tests/wpt-core-team.

I've looked over https://github.com/cta-wave/WMAS and I see two parts: infrastructure changes and new tests. I assume you're proposing to integrate both of these, or would either in isolation be valuable?

Infrastructure

There are a few branches there, but it looks like wmas2018 is the main one and it branched from this repo at commit 4bdeca6b451519a7f60f592468600e0a6cbfc42b. Using git diff --diff-filter=AM (to ignore deletions) it looks like the important changes are:

• tools/wave: A server to assist the run of the web platform tests in a single window
• changes to resources/testharnessreport.js also for this purpose?
• a WaveProxyHandler in tools/wptserve/wptserve/handlers.py, also part of the same?

Is running in a browser that doesn't support tabs the whole scope of these changes? Can you explain what the purpose of the node server here is, could the same not be achieved with wptserve?

New tests

I see some new tests in https://github.com/cta-wave/WMAS/tree/wmas2018-tests/wave-extra.

Any tests that are asserting things already required by existing web specs would be fine to just submit as pull requests, and "Devices MUST support deleting a cookie" is probably such a test, although I'd expect there to be existing cookie deletion tests somewhere.

It looks like a bunch of the tests are manual, which is understandable for a test that says "Please restart the device!" :) Manual tests won't be run and thus submitting them has pretty low impact. If you can find a good place to put these tests I think that'd be fine.

Most of the tests seem to be of the type "Devices MUST support at least 100 total cookies" or similar. These are a bit like stress tests, although not pushing the limits very far. All but one of the tests are about cookies, so pinging the lone reviewer of cookies in wpt, @mikewest, for thoughts on adding such tests. My own sense is that as long as the tests are well written and don't push the limits very far, they're fine. Tests that would be very slow or use a lot of resources I'd be more wary about.

Are there more new tests on some other branch?

Thanks @foolip the infrastructure changes are more important I would propose to handle them separately from the new tests. The main branch wmas2018 is the relevant branch for us. Below are comments to your questions

tools/wave: A server to assist the run of the web platform tests in a single window

Exactly we are targetting embedded devices like TVs and STBs where you can only run tests in a single window. Therefore, we shifted part of the test runner logic to the server which is written in Node.js. The Test Runner offers also a companion page where you can monitor/control the test session on the DUT. You can also list, view and compare test sessions and generate reports (using wptreport tool) through the companion page.

changes to resources/testharnessreport.js also for this purpose?

Yes, we modified this file in order to report the results of the current test to the Server and ask for the next test file in the current session.

a WaveProxyHandler in tools/wptserve/wptserve/handlers.py, also part of the same?

Yes, basically it forwards requests to the Node.js Server to avoid cross-origin request.

Is running in a browser that doesn't support tabs the whole scope of these changes?

Yes, browsers that don't support tabs are in scope but as I mentioned above, in CTA WAVE we are mainly targetting TVs and STBs.

Can you explain what the purpose of the node server here is, could the same not be achieved with wptserve?

The Server implements the whole logic to managing test runs (sessions) and generating the test reports. The tests are served as before via wptserve. The changes we made in resources/testharnessreport.js are responsible for the communication with the Server.

We can handle new tests in a seperate issue and focus in this RFC only on the infrastructure.

Most of the tests seem to be of the type "Devices MUST support at least 100 total cookies" or similar. These are a bit like stress tests, although not pushing the limits very far. All but one of the tests are about cookies, so pinging the lone reviewer of cookies in wpt, @mikewest, for thoughts on adding such tests.

I guess they're fine, with the caveat that landing these tests in WPT is not going to create an obligation on behalf of browser vendors to continue supporting whatever limits are being validated. Folks seem enthusiastic about imposing various and sundry limitations/expirations on the ability to use document.cookie, for example. Likewise, the non-HTTPS variants might stop working if secure context restrictions expand.

It's unlikely that vendors will feel bound to keep these tests passing, and therefore likely that they'll be modified by one of the vendors as behaviors evolve. If you need these to be a stable conformance test for hardware you verify, it's probably a better idea to keep them in a repository you control.

We can handle new tests in a seperate issue and focus in this RFC only on the infrastructure.

@louaybassbouss that sounds good, can you rename this PR to make the limited scope clear? (The branch and file names already seem fine.)

My overall question about the running infrastructure is if the same could be achieved by modifications to wptrunner / wptserve implemented in Python, and what the drawbacks of doing that would be.

Also, how is communication with the TV/STB established in the first place? If you're using WebDriver I'd be optimistic that you could rejig the existing code base to avoid creating that extra window. However, if there's some manual step involved in connecting the test runner to the device under test, and there's no control protocol like WebDriver involved, then the changes would probably be more involved.

Most of the inner working is documented in the issue web-platform-tests/wpt#16214

My understanding is that WebDriver is not used, and it'd be difficult to retro-fit the proposed model into wptrunner. I think it's fine to have an additional server backend to control the test execution but I hope it'd be implemented in Python instead of Node.js.

Looking at @jgraham's comment in web-platform-tests/wpt#16214 (comment) - who will own the new infrastructure and tests?

I'd also like to see more details in "Risks". Adding new infrastructure, new dependencies (Node.js?), new tests, and new users to the project doesn't seem like it would be without risk.

@louaybassbouss do you have a clear idea of what extra details we're asking for?

@foolip sorry for the late reply on this yes for me it is clear what are the open questions and what to do from our side let me summarize:

• Update risk section as proposed by @zcorpan and mention the new dependency from Node.js.
• Provide more description of the workflow and the Communication with TV/STB and your comment about WebDriver
  • BTW there is an online hosted version of the WMAS2018 test runner (what is currently in the WMAS master branch on GitHub) just open this Link https://webapitests2018.ctawave.org in a Browser your choice and click on "Continue" button. on the next page, you can scan the QR Code to open the results page on a companion device and then hit enter to start the tests. Please let me know if you have any question on this.
  • [UPDATE]: This [Usage Guide] provides more description of the workflow and the Communication with TV/STB.
• Who will be the owner?
  • CTA will own the new infrastructure Changes. Please guide me what information you need to move forward on this.

Are there other points I missed?

@louaybassbouss that all sounds right.

I think the Node.js dependency is going to be the primary concern here, since we don't have any node dependencies so far, at least no runtime dependencies. The core functionality of being able to run on a browser that doesn't support tabs is one I think ought to be possible to support in wptrunner+wptserve with modest modifications, so I'd suggest exploring that, as the ongoing maintenance burden of that ought to be much smaller than a new server on a new tech stack.

This is not to say that a Node.js dependency is absolutely out of the question, just that it requires very strong justification.

Should we close this, as the RFC in its current state (or remotely close to it) isn't something we will accept? Per the above, we need a lot more detail in the RFC to evaluate what exactly is proposed here.

FWIW I'm happy to leave this open for now, assuming that there's a path to agreement here.

@jgraham @gsnedders @foolip I update the risk section concerning the issue with the Node.js dependency. We need a final decision if Node.js is still an option or not. If not, the only option is to port the code to Python and integrate it into wpt-* tools, but this option is associated with additional effort and requires internal approval in the project.

@louaybassbouss I don't think that a runtime Node.js dependency is an option at this point, at least not for functionality that can plausibly be supported in the existing wptrunner code base.

Others may speak up if they disagree, of course.

Yes, I agree with that. Taking a second HTTP server based on a tech stack that is otherwise not used in the project is just too much in terms of the additional maintaince burden and increased difficulty of deployment. However given the implementation issues are fixed, I'm positive about landing the featureset proposed by this change; there's no implied lack of desire to support the use cases here.

@foolip @jgraham @gsnedders thank you for your feedback and support. Following your recommendation, we are going to covert the Node.js source code to Python and integrate it into the wpt-* tools. We would like to use this channel for discussion, can we keep this PR open?

Glad to hear this is progressing! Continuing here works, or https://github.com/web-platform-tests/wpt/issues/new would also make sense. The upside of the latter is we can apply the appropriate labels, but it's a small upside.

Thanks @foolip, yes we will then create concrete new issues with appropriate labels.

Dear all, I am happy to announce that we finished the work on porting the Node.js code into Python. I just created a PR in the WPT repository. For more details please refer to the description of the PR. Please let me know if you have questions or you need clarifications.

@louaybassbouss given that the associated PR for this (web-platform-tests/wpt#21323) has now merged, can you update this RFC to reflect the current situation? I think important points are:

• the runner is in python, no longer Node.js
• we've taken steps to make the runner isolated from the rest of the WPT stack, to minimize the burden on WPT tooling
• (risk) whilst we have integration tests, the WPT infra team will not commit to maintaining this so if nobody else does as WPT tooling changes then it may break and be left behind

@louaybassbouss ping, are you able to schedule some time to update this RFC to reflect the current situation?

@stephenmcgruer I will do it ASAP I need to sync with WAVE group about it before I comment. I that ok for you?

@stephenmcgruer I will do it ASAP I need to sync with WAVE group about it before I comment. I that ok for you?

Sounds great, thanks :)

@stephenmcgruer sorry for the delay I just updated the RFC. Please check and let us know if you still have questions. @JohnRiv is the update ok for you?

Thanks @louaybassbouss, I'm good with the content in the update.

LGTM. Will merge on July 24th (one week after latest update).