PSA: Move third_party/WebKit/LayoutTests to third_party/blink/web_tests on November 25-26

[TAMURA, Kent]

We're going to move third_party/WebKit/LayoutTests to third_party/blink/web_tests. The plan is to close the tree on Nov. 25 at 2:00pm PST, on Nov. 26 at 7:00am JST, and we open the tree in a few hours if we have no unexpected issues.

We just rebase and merge the following two CLs.

https://chromium-review.googlesource.com/c/chromium/src/+/1328627

https://chromium-review.googlesource.com/c/infra/infra/+/1331192

If you have any concerns, please let me know.

--

TAMURA Kent
Software Engineer, Google

[Morten Stenshorne]

Are there any plans to shorten the path to WPT tests?

third_party/WebKit/LayoutTests/external/wpt/ (or
third_party/blink/web_tests/external/wpt/) is rather tedious. Given that
the default now is to write WPT tests, shouldn't we move all the *other*
tests into something slightly more inconvenient (e.g. web_tests/legacy/
or web_tests/blink/), and put WPT tests directly under the web_tests/
root, so that e.g. the WPT test css/css-multicol/multicol-gap-000.xht is
found under third_party/blink/web_tests/css/css-multicol/ in our repo
(and e.g. good old
third_party/WebKit/LayoutTests/fast/inline/inline-body-crash.html moved
under third_party/blink/web_tests/blink/fast/inline/)?

--
Morten Stenshorne, Software developer,
Blink/Layout, Google, Oslo, Norway

[PhistucK]

How about .../web_tests/internal/ for non-web-platform-tests tests?

☆PhistucK

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/ozzizhudibe6.fsf%40aeneas.osl.corp.google.com.

[Jeremy Roman]

I'm not sure it's desirable to tie rewriting all TestExpectations, virtual test suites, and other things that use LayoutTests-relative paths, to what is already likely to be a fairly large and tricky change.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CABc02_JSRhdB8ZefXsoka8vMEKRRy0Dz5%2B3Yn1UMe6%2Bi9%2B8nqw%40mail.gmail.com.

[Robert Ma]

First of all, thanks for doing the hard work, Kent!

I'd like to second Morten's suggestion of taking the chance to shorten WPT paths. This is also and idea Philip (cc'ed) and I have been talking about.

Jeremy's response actually pointed out the two important aspects of this problem:

* Desirability: we would like to shorten the path to WPT because of two main reasons:

1) Now that with CSS tests merged into WPT, there is really only one "external" suite of web test, i.e. WPT. And the web-platform-tests group hopes to keep it that way (WPT is expected to be the single suite for everything except ECMAScript & WebGL). Therefore, the "external" layer is unnecessary, only making the tree deep and the path tedious.

2) We would like to promote WPT as the default suite for web tests. And "external" not only makes the path less developer friendly, but also sends the wrong signal, especially to newcomers ("external" implies some imported tests that shouldn't be modified, whereas in fact WPT is being synced and is a first-class citizen). Furthermore, it'd make our position clearer if we could move all non-WPT tests into internal/ legacy/, but that might cause too much disturbance (see below).

* Complexity & timing: I think there are three major additional concerns around changing the directory structure within LayoutTests (web_tests): result history, expectation files, and virtual suite definitions, since all of them use the relative path from LayoutTests as the identifier for a test.

I would argue that getting rid of "external/" (i.e. promoting external/wpt to wpt/) is not a big issue. The directory structure of WPT itself is already quite fluid; we've already had quite a few large-scale renaming in WPT, so there's not too much reliable result history to begin with. There are around 3K lines of expectations for WPT, most of which are automatically generated by the importer. And there are only 41 virtual suites in WPT. Moving the non-WPT tests into internal/, on the other hand, is understandably more controversial because of their different natures on these regards.

I agree moving LayoutTests is already a tricky change, but I'd say this is exactly the reason why it'd be a good timing to rejig the directory structure -- the additional complexity and risk is small compared to moving the whole LayoutTests, and we already have a closed tree (instead of closing the tree again in the future just to tweak the directory structure).

To summarize, I'm proposing to take the chance to rename external/wpt to wpt. I can help to prepare the change and volunteer to stay on call to work with Kent when the tree is closed in case anything goes wrong. What do you think, Kent?

Please feel free to raise any other concerns or different opinions.

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/ozzizhudibe6.fsf%40aeneas.osl.corp.google.com.

[Philip Jägenstedt]

Thanks to Kent-san for working on this indeed! Seeing the previous call for volunteers we actually put this on our Ecosystem Infra Q4 OKRs, and it's great to see the work getting done without our investment of time.

As Robert says, we have talked a fair bit in our team about various ways of renaming to make WPT more prominent and obviously the default test suite, and had thought of a renaming as an opportunity to do this. Within the limitation of a single test root, third_party/blink/web_tests/wpt does seem like the ideal directory name.

So, if we could pitch in and make the external/wpt -> wpt rename part of this we'd much appreciate it.

For everything else, there's really a lot of types of tests in LayoutTests, some of which could be part of wpt and some that probably never could. Rather than moving them all into internal/, I think we might end up going through them at a more granular level, perhaps putting them into categories like run_like_wpt_but_dont_export/, paint_invalidation/, style_recalc_internal/ and similar. (Made up names, obviously.) With a good README, we can still end up in a situation where wpt/ is the directory that people naturally find when adding tests.

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/ozzizhudibe6.fsf%40aeneas.osl.corp.google.com.

[TAMURA, Kent]

I agree with the idea of removing 'external/', but I won't do it as a part of the LayoutTests move.

 - Moving LayoutTests itself is a high-risk change.  Try bots are almost useless, and there are several external dependencies. I'd like to avoid additional complexity and risk.

 - Almost no benefit from doing the LayoutTest move and removing 'external/' at the same time.  Unlike moving source code, I don't think we should avoid massive changes for cleaner revision log.

We should remove 'external/' before or after the LayoutTests move. If the fragile TestExpectations makes the task harder, we may introduce a transient hack to ignore 'external/' in TestExpectations entries internally.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAARdPYeN6eVTs6k7T3doDcf-F3bRKzwM4DR4dKiUS9NXwyPXhA%40mail.gmail.com.

[Robert Ma]

Thanks for the reply! CIL

On Tue, Nov 13, 2018 at 7:52 PM TAMURA, Kent <tk...@chromium.org> wrote:

I agree with the idea of removing 'external/', but I won't do it as a part of the LayoutTests move.

 - Moving LayoutTests itself is a high-risk change.  Try bots are almost useless, and there are several external dependencies. I'd like to avoid additional complexity and risk.

I didn't know try bots would be almost useless. That's indeed a significant risk and I can see why restricting the directory meanwhile is not favorable.

 

 - Almost no benefit from doing the LayoutTest move and removing 'external/' at the same time.  Unlike moving source code, I don't think we should avoid massive changes for cleaner revision log.

I was more concerned about having to close the tree again just to move external/wpt because of massive merge conflicts, but it's probably not too bad (and I might be able to land it without closing the tree, perhaps by making use of wpt-importer).

We should remove 'external/' before or after the LayoutTests move. If the fragile TestExpectations makes the task harder, we may introduce a transient hack to ignore 'external/' in TestExpectations entries internally.

This is a brilliant idea and should make the transition much easier! Let me look into it.

[TAMURA, Kent]

We're going to move LayoutTests on the next Sunday-Monday as planned.

I recommend NOT to make new CLs including many changes in LayoutTests until the next week. We won't provide a tool to help rebase.

[TAMURA, Kent]

We have moved LayoutTests, and the tree is open now.

If you find any issues due to the move, please help to fix them.

On Tue, Nov 13, 2018 at 11:51 AM TAMURA, Kent <tk...@chromium.org> wrote: