Re: [webkit-dev] Upstreaming Tests from WebKit to Web Platform Tests
On Mon, Feb 6, 2017 at 7:32 PM Ryosuke Niwa wrote: > On Mon, Feb 6, 2017 at 4:22 PM, youenn fablet wrote: > > It seems we agree in moving this forward. Any objection? > If so, let's start with small practical steps, something like a script to > automatically generate WPT PR requests from a WebKit repo. > > > I think we need to first agree on where to put new tests. If we're placing > next to existing tests inside LayoutTests/imported/w3c/web-platform-tests/ > then identifying those new tests will be an issue. > > Also, there needs to be a mechanism to detect these new tests that have > been merged into upstream when we're re-importing. It's possible for tests > newly added in WebKit to be renamed, moved, or otherwise modified before we > re-import them back. > > All these situations need to be carefully thought out first. > If it might be of any use, here's a doc where we hash out a lot of question for 2-way sync in Chromium: https://docs.google.com/document/d/1JgPTyIWmjlXyhyatiZ9A6fSbUQPjCyM26budEY90VDE/edit?usp=sharing Linked from that is also an older doc with a lot of discussion: https://docs.google.com/document/d/1JOcZsURB3ITsRtundqkpVVDrqo6x-_jdif7s_0rLJD0/edit?usp=sharing When it comes to identifying which commits to export next, we look for the most recent exported commit in web-platform-tests, map that to a Chromium commit, and then inspect all later commits that touch LayoutTests/external/wpt/. This scheme only works if commits are always exported in order. Note also that commits before the most recent import can also be candidates, because of imports racing with test changes in CQ. Automatic imports are also blocked on all exports being finished, to not (temporarily) revert changes. Happy to provide more details on design and trade-offs if useful to you. Regardless, I'm quite excited to see this happening in WebKit. ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Upstreaming Tests from WebKit to Web Platform Tests
> 6 февр. 2017 г., в 12:28, Ryosuke Niwa написал(а): > > The concern I've heard about is not how run-webkit-tests run the tests. It's > about how a test is opened inside a browser, DRT, WTR while debugging. +1 I think that making web platform tests more practical for engineers to work with should be a pre-requisite for deeper integration. From tools perspective, cleaning up the way we get supporting scripts for web platform tests would be quite beneficial too. It is very hard to reason about what happens in a merge that just updates .tar.gz files or a SHA hash. - Alexey ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Upstreaming Tests from WebKit to Web Platform Tests
On Mon, Feb 6, 2017 at 4:22 PM, youenn fablet wrote: > It seems we agree in moving this forward. Any objection? > If so, let's start with small practical steps, something like a script to > automatically generate WPT PR requests from a WebKit repo. > I think we need to first agree on where to put new tests. If we're placing next to existing tests inside LayoutTests/imported/w3c/web-platform-tests/ then identifying those new tests will be an issue. Also, there needs to be a mechanism to detect these new tests that have been merged into upstream when we're re-importing. It's possible for tests newly added in WebKit to be renamed, moved, or otherwise modified before we re-import them back. All these situations need to be carefully thought out first. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Upstreaming Tests from WebKit to Web Platform Tests
I filed https://bugs.webkit.org/show_bug.cgi?id=167911. Le lun. 6 févr. 2017 à 16:22, youenn fablet a écrit : > It seems we agree in moving this forward. Any objection? > If so, let's start with small practical steps, something like a script to > automatically generate WPT PR requests from a WebKit repo. > > Le lun. 6 févr. 2017 à 12:28, Ryosuke Niwa a écrit : > > > On Monday, February 6, 2017, youenn fablet wrote: > > The two complaints I heard against testharness.js are: > > - They are less easy to debug as test harness.js does not print out > messages for each assert. There might be room for updating testharness to > support that > - Async tests are less easy to write. While this is probably true, > testharness has support for promise-based tests which are not verbose. > WPT has also some benefits of its own, like the possibility to run easily > the same tests in worker/window environments, the flexible python-based > server... > > One complaint I heard against WPT tests is that they require being run > behind a server. > This is becoming more and more true. > There is still a large portion of tests that could be run locally if we > update the paths to test harness.js/testharnessreport,js. > I am not a big fan of modify any WPT test but maybe we should consider > switching back to modifying these paths at import and export time. > > > We should probably do this for the sake of making tests more debuggable. > Having to run a HTTP/HTTPS server makes testing WebCore against a test > needlessly harder especially for things like core DOM API which doesn't > require any network stack. > > > Or we could make our tooling better to also handle LayoutTests/http tests > more easily. > In any case, our CI infrastructure should run WPT tests as closely as > specified by WPT. > > > The concern I've heard about is not how run-webkit-tests run the tests. > It's about how a test is opened inside a browser, DRT, WTR while debugging. > > > I would tend to do the following: > - Write/submit WPT tests in WebKit as regular WebKit testharness.js layout > tests through bugzilla > - At commit queue time, automatically create a PR to WPT GitHub containing > the changes of the WebKit patch > > > I think commit queue time is too late. Remember that not everyone uses > commit queue. Also, if there was any issue with a test, we should be able > to tell that prior to landing the patch. > > > Ideally, having one EWS bullet/bot capturing/handling PR status would be > very good. > Knowing the state of a test from various browsers would be helpful at > review time. > > But this might require some extra work to support PR updates and so on. > CQ time seems like a reasonable target for step 1. > Well, step 0 might be to have a script that committers would run > themselves locally. > > > Again, many people don't use CQ. What do those people do? Manually run a > script? I don't think that's a workable solution. There are many people > just land patches from Xcode UI for example. > > In general, I don't think it's acceptable to introduce any extra step for > WebKit contributors. Let it be running a new script, potentially fix an > issue in GitHub PR, etc... > > There should be absolutely no new thing contributor has to run or monitor. > That should be the minimum bar for this upstreaming system to exist. > > Again, we can't even keep up with test failures on our own bots, and > people frequently land patches with broken tests or tests that fail on some > bots. There is no way we can succeed by introducing yet another step / > place humans has to care. That's simply unacceptable. > > - Ask committer to fix the WPT PR should there be any issue with it > (committer being informed of such issues through bugzilla). > > > I think this is too much of an overhead / a burden for WebKit > contributors. Now patch contributors need to worry about EWS, > build.webkit.org, and some random PR that gets created on Github failing. > > > Contributors would just need to worry about monitoring bugzilla, like they > are doing for EWS/CQ. > > > It depends on what kind of "monitoring" is required. > > > The number of conflicts should be fairly small hopefully. > I am not sure that we can come up with a solution that would handle > conflicts automatically. > > > In the case of a conflict, the patch should be rejected in EWS & CQ just > the same way a WebKit patch conflict would. > > - R. Niwa > > > -- > - R. Niwa > > ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Upstreaming Tests from WebKit to Web Platform Tests
It seems we agree in moving this forward. Any objection? If so, let's start with small practical steps, something like a script to automatically generate WPT PR requests from a WebKit repo. Le lun. 6 févr. 2017 à 12:28, Ryosuke Niwa a écrit : > > On Monday, February 6, 2017, youenn fablet wrote: > > The two complaints I heard against testharness.js are: > > - They are less easy to debug as test harness.js does not print out > messages for each assert. There might be room for updating testharness to > support that > - Async tests are less easy to write. While this is probably true, > testharness has support for promise-based tests which are not verbose. > WPT has also some benefits of its own, like the possibility to run easily > the same tests in worker/window environments, the flexible python-based > server... > > One complaint I heard against WPT tests is that they require being run > behind a server. > This is becoming more and more true. > There is still a large portion of tests that could be run locally if we > update the paths to test harness.js/testharnessreport,js. > I am not a big fan of modify any WPT test but maybe we should consider > switching back to modifying these paths at import and export time. > > > We should probably do this for the sake of making tests more debuggable. > Having to run a HTTP/HTTPS server makes testing WebCore against a test > needlessly harder especially for things like core DOM API which doesn't > require any network stack. > > > Or we could make our tooling better to also handle LayoutTests/http tests > more easily. > In any case, our CI infrastructure should run WPT tests as closely as > specified by WPT. > > > The concern I've heard about is not how run-webkit-tests run the tests. > It's about how a test is opened inside a browser, DRT, WTR while debugging. > > > I would tend to do the following: > - Write/submit WPT tests in WebKit as regular WebKit testharness.js layout > tests through bugzilla > - At commit queue time, automatically create a PR to WPT GitHub containing > the changes of the WebKit patch > > > I think commit queue time is too late. Remember that not everyone uses > commit queue. Also, if there was any issue with a test, we should be able > to tell that prior to landing the patch. > > > Ideally, having one EWS bullet/bot capturing/handling PR status would be > very good. > Knowing the state of a test from various browsers would be helpful at > review time. > > But this might require some extra work to support PR updates and so on. > CQ time seems like a reasonable target for step 1. > Well, step 0 might be to have a script that committers would run > themselves locally. > > > Again, many people don't use CQ. What do those people do? Manually run a > script? I don't think that's a workable solution. There are many people > just land patches from Xcode UI for example. > > In general, I don't think it's acceptable to introduce any extra step for > WebKit contributors. Let it be running a new script, potentially fix an > issue in GitHub PR, etc... > > There should be absolutely no new thing contributor has to run or monitor. > That should be the minimum bar for this upstreaming system to exist. > > Again, we can't even keep up with test failures on our own bots, and > people frequently land patches with broken tests or tests that fail on some > bots. There is no way we can succeed by introducing yet another step / > place humans has to care. That's simply unacceptable. > > - Ask committer to fix the WPT PR should there be any issue with it > (committer being informed of such issues through bugzilla). > > > I think this is too much of an overhead / a burden for WebKit > contributors. Now patch contributors need to worry about EWS, > build.webkit.org, and some random PR that gets created on Github failing. > > > Contributors would just need to worry about monitoring bugzilla, like they > are doing for EWS/CQ. > > > It depends on what kind of "monitoring" is required. > > > The number of conflicts should be fairly small hopefully. > I am not sure that we can come up with a solution that would handle > conflicts automatically. > > > In the case of a conflict, the patch should be rejected in EWS & CQ just > the same way a WebKit patch conflict would. > > - R. Niwa > > > -- > - R. Niwa > ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Upstreaming Tests from WebKit to Web Platform Tests
On Monday, February 6, 2017, youenn fablet wrote: > The two complaints I heard against testharness.js are: > >> - They are less easy to debug as test harness.js does not print out >>> messages for each assert. There might be room for updating testharness to >>> support that >>> - Async tests are less easy to write. While this is probably true, >>> testharness has support for promise-based tests which are not verbose. >>> WPT has also some benefits of its own, like the possibility to run >>> easily the same tests in worker/window environments, the flexible >>> python-based server... >>> >>> One complaint I heard against WPT tests is that they require being run >>> behind a server. >>> This is becoming more and more true. >>> There is still a large portion of tests that could be run locally if we >>> update the paths to test harness.js/testharnessreport,js. >>> I am not a big fan of modify any WPT test but maybe we should consider >>> switching back to modifying these paths at import and export time. >>> >> >> We should probably do this for the sake of making tests more debuggable. >> Having to run a HTTP/HTTPS server makes testing WebCore against a test >> needlessly harder especially for things like core DOM API which doesn't >> require any network stack. >> > > Or we could make our tooling better to also handle LayoutTests/http tests > more easily. > In any case, our CI infrastructure should run WPT tests as closely as > specified by WPT. > The concern I've heard about is not how run-webkit-tests run the tests. It's about how a test is opened inside a browser, DRT, WTR while debugging. I would tend to do the following: >>> - Write/submit WPT tests in WebKit as regular WebKit testharness.js >>> layout tests through bugzilla >>> - At commit queue time, automatically create a PR to WPT GitHub >>> containing the changes of the WebKit patch >>> >> >> I think commit queue time is too late. Remember that not everyone uses >> commit queue. Also, if there was any issue with a test, we should be able >> to tell that prior to landing the patch. >> > > Ideally, having one EWS bullet/bot capturing/handling PR status would be > very good. > Knowing the state of a test from various browsers would be helpful at > review time. > > But this might require some extra work to support PR updates and so on. > CQ time seems like a reasonable target for step 1. > Well, step 0 might be to have a script that committers would run > themselves locally. > Again, many people don't use CQ. What do those people do? Manually run a script? I don't think that's a workable solution. There are many people just land patches from Xcode UI for example. In general, I don't think it's acceptable to introduce any extra step for WebKit contributors. Let it be running a new script, potentially fix an issue in GitHub PR, etc... There should be absolutely no new thing contributor has to run or monitor. That should be the minimum bar for this upstreaming system to exist. Again, we can't even keep up with test failures on our own bots, and people frequently land patches with broken tests or tests that fail on some bots. There is no way we can succeed by introducing yet another step / place humans has to care. That's simply unacceptable. - Ask committer to fix the WPT PR should there be any issue with it >>> (committer being informed of such issues through bugzilla). >>> >> >> I think this is too much of an overhead / a burden for WebKit >> contributors. Now patch contributors need to worry about EWS, >> build.webkit.org, and some random PR that gets created on Github failing. >> > > Contributors would just need to worry about monitoring bugzilla, like they > are doing for EWS/CQ. > It depends on what kind of "monitoring" is required. > The number of conflicts should be fairly small hopefully. > I am not sure that we can come up with a solution that would handle > conflicts automatically. > In the case of a conflict, the patch should be rejected in EWS & CQ just the same way a WebKit patch conflict would. - R. Niwa -- - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Upstreaming Tests from WebKit to Web Platform Tests
The two complaints I heard against testharness.js are: > - They are less easy to debug as test harness.js does not print out > messages for each assert. There might be room for updating testharness to > support that > - Async tests are less easy to write. While this is probably true, > testharness has support for promise-based tests which are not verbose. > WPT has also some benefits of its own, like the possibility to run easily > the same tests in worker/window environments, the flexible python-based > server... > > One complaint I heard against WPT tests is that they require being run > behind a server. > This is becoming more and more true. > There is still a large portion of tests that could be run locally if we > update the paths to test harness.js/testharnessreport,js. > I am not a big fan of modify any WPT test but maybe we should consider > switching back to modifying these paths at import and export time. > > > We should probably do this for the sake of making tests more debuggable. > Having to run a HTTP/HTTPS server makes testing WebCore against a test > needlessly harder especially for things like core DOM API which doesn't > require any network stack. > Or we could make our tooling better to also handle LayoutTests/http tests more easily. In any case, our CI infrastructure should run WPT tests as closely as specified by WPT. > I would tend to do the following: > - Write/submit WPT tests in WebKit as regular WebKit testharness.js layout > tests through bugzilla > - At commit queue time, automatically create a PR to WPT GitHub containing > the changes of the WebKit patch > > > I think commit queue time is too late. Remember that not everyone uses > commit queue. Also, if there was any issue with a test, we should be able > to tell that prior to landing the patch. > Ideally, having one EWS bullet/bot capturing/handling PR status would be very good. Knowing the state of a test from various browsers would be helpful at review time. But this might require some extra work to support PR updates and so on. CQ time seems like a reasonable target for step 1. Well, step 0 might be to have a script that committers would run themselves locally. > - Ask committer to fix the WPT PR should there be any issue with it > (committer being informed of such issues through bugzilla). > > > I think this is too much of an overhead / a burden for WebKit > contributors. Now patch contributors need to worry about EWS, > build.webkit.org, and some random PR that gets created on Github failing. > Contributors would just need to worry about monitoring bugzilla, like they are doing for EWS/CQ. The number of conflicts should be fairly small hopefully. I am not sure that we can come up with a solution that would handle conflicts automatically. > The fact people have to worry about EWS & build.webkit.org separately is > bad enough. We shouldn't be introducing yet another place people have to > monitor / care about. > > - R. Niwa > > ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] CSS Parse error in element.
Hi Atul, I second Alex's suggestion (perhaps followed by HTMLLinkElement::process() and other places in that file that refer to `hrefAttr`). If you have a test case online, I could try to take a look and maybe provide more guidance. Cheers :) Yoav On Fri, Feb 3, 2017 at 9:19 PM Alex Christensen wrote: > I would start looking at HTMLLinkElement::parseAttribute. > LinkHeader.cpp contains parsers for link headers, which are related. Yoav > knows more about those. Those parsers ought to be united more. > > On Feb 3, 2017, at 1:17 AM, Atul Sowani wrote: > > At present I am focusing on CSSParser::findURI() particularly > and CSSParser::realLex() other related functionality in CSSParser.cpp > - hope I am on right track. ;-) > > Please let me know if I should be looking at some other functionality as > well to resolve this issue. > > Thanks! > Atul. > > On Fri, Feb 3, 2017 at 2:33 PM, Atul Sowani wrote: > > Hi, > > I came across an issue in qtwebkit CSS parser while working on a PhantomJS > crash. The issue seems to be with parsing of > type elements in an HTML page. What I observed is that the parser is trying > to interpret the value for href given inside double-quotes. The value > contains a "-" (e.g. "http://some.domain.com/some-page-etc-etc";). The "-" > sign is being interpreted as minus and then things go wrong. In another > case I found that "\g" embedded in the value (e.g. " > http://some.domain.com/some-page/global/something";) is also creating > issues. In essence, the parser is trying to interpret the value, which I > believe, it should not. > > I am willing to dive further into it to debug and fix the issue, but > looking at the complexity and size of WebCore, I think I would benefit a > lot to expedite a fix, if I could get some tips about which code > area/functionality I should specifically focus in the WebCore. Looking > forward to some help in this regard. > > Thanks, > Atul. > > > ___ > webkit-dev mailing list > webkit-dev@lists.webkit.org > https://lists.webkit.org/mailman/listinfo/webkit-dev > > ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Upstreaming Tests from WebKit to Web Platform Tests
On Sun, Feb 5, 2017 at 6:54 PM, Philip Jägenstedt wrote: > FWIW, in Blink we stopped rewriting the testharness.js paths before > switching to wptserve, by instead rewriting those URLs only when running > LayoutTests: > https://cs.chromium.org/chromium/src/content/shell/ > renderer/layout_test/blink_test_runner.cc?type=cs&q= > content/shell/renderer/layout_test/blink_test_runner.cc&l=221 > > So, we know that it's possible to run a lot of the tests without wptserve > without modifying the tests. However, trying to list all the tests that do > or don't need wptserve seems like a lot of work, and I think we're now > using wptserve for all tests. > For us, the biggest concern is the ability to open it up on browser and attach debugger easily so I don't think rewriting paths like that in DRT/WTR or webkitpy would really address the problem. One approach I came up during the meeting was to run a test with and without a server. If the results match, there is a good chance the test doesn't require a server. We can put that meta data somewhere (e.g. we can insert a link/meta element next to where testharness.js is imported) so that the information is readily available. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Upstreaming Tests from WebKit to Web Platform Tests
On Sun, Feb 5, 2017 at 5:04 PM, youenn fablet wrote: > I too am a big proponent of us moving more and more towards WPT. > As part of the streams and fetch API implementation, most of the related > functional tests have been made as WPT. > The benefits of having tests being improved and updated largely outweighs > the testharness.js initial cost. > Somehow, these tests are becoming part of their related specs. > > The two complaints I heard against testharness.js are: > - They are less easy to debug as test harness.js does not print out > messages for each assert. There might be room for updating testharness to > support that > - Async tests are less easy to write. While this is probably true, > testharness has support for promise-based tests which are not verbose. > WPT has also some benefits of its own, like the possibility to run easily > the same tests in worker/window environments, the flexible python-based > server... > > One complaint I heard against WPT tests is that they require being run > behind a server. > This is becoming more and more true. > There is still a large portion of tests that could be run locally if we > update the paths to test harness.js/testharnessreport,js. > I am not a big fan of modify any WPT test but maybe we should consider > switching back to modifying these paths at import and export time. > We should probably do this for the sake of making tests more debuggable. Having to run a HTTP/HTTPS server makes testing WebCore against a test needlessly harder especially for things like core DOM API which doesn't require any network stack. I would tend to do the following: > - Write/submit WPT tests in WebKit as regular WebKit testharness.js layout > tests through bugzilla > - At commit queue time, automatically create a PR to WPT GitHub containing > the changes of the WebKit patch > I think commit queue time is too late. Remember that not everyone uses commit queue. Also, if there was any issue with a test, we should be able to tell that prior to landing the patch. - Ask committer to fix the WPT PR should there be any issue with it > (committer being informed of such issues through bugzilla). > I think this is too much of an overhead / a burden for WebKit contributors. Now patch contributors need to worry about EWS, build.webkit.org, and some random PR that gets created on Github failing. The fact people have to worry about EWS & build.webkit.org separately is bad enough. We shouldn't be introducing yet another place people have to monitor / care about. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
