Previously you don't need to specify anything in BUILD.gn is because we are downloading the whole "web_tests" folder. Now we want to run legacy tests and wpt in different steps. Under "web_tests/virtual" we have baselines for wpt and pure virtual legacy tests, we can list the folders under "virtual/prefix" for each virtual test suite, which will make the dependency very large(yes), and we need make such change each time when we add new virtual suites. I don't think this is something blink devs want to do.
The discussion is at the beginning of the crbug. So pls scroll back to #c1, and read from there. thanks, Weizhong On Thu, Jun 2, 2022 at 12:29 PM Domenic Denicola <dome...@chromium.org> wrote: > > > On Thu, Jun 2, 2022 at 3:21 PM Weizhong Xia <weizh...@google.com> wrote: > >> Folks >> >> I'm sorry to see this has caused inconvenience, and sorry for being late >> in response to this, due to the same reason Dom had. >> > > Thanks for responding and listening to our concerns! > > >> >> The reason to move baselines to one central place at this point is to >> make it possible to specify dependency in BUILD.gn. >> > > I don't quite understand this. I've had to work with WPTs and WPT > expectations my entire time working on Chromium. I've never had to "specify > dependency in BUILD.gn"; I don't really know what that means. I'd love to > hear more (or be referred to a doc explaining the issue), so that I > understand why we're making the sacrifice we're making. (The linked bug > isn't very understandable for me, unfortunately. I can't even understand > enough to find the part you mentioned where you discussed different > approaches.) > > I'm also unsure how the decision was weighed. Is the population of people > specifying dependency in BUILD.gn very large, so that their needs are > outweighing those of the Chromium developers working with web platform > tests? > > >> This is part of the work to use upstream wptrunner to run wpt tests. In >> crbug/1299834 <https://crbug.com/1299834> we have tried to discuss some >> different approaches. I would say this is the least disruptive way. For >> blink engprod, I think to make blink devs happy is our top priority. >> >> For the long term, we can definitely move this back once we have separate >> legacy tests and wpt into different folders. I can also reach out to you >> guys to better understand your needs. >> >> For the folders Dom mentioned, I can check to see if any of that can be >> removed. I understand the README is needed for virtual test suites. >> >> Cheers, Weizhong >> >> On Wednesday, June 1, 2022 at 6:06:56 AM UTC-7 yoav...@chromium.org >> wrote: >> >>> +1 from me as well. I was similarly caught by surprise by this change >>> (during reviews for webexposed changes), and am similarly not seeing the >>> upside for this. >>> >>> While I'm sure this is a change that was meant to be a positive one, I'd >>> love to better understand the reasoning, and whether the current situation >>> is a temporary one, or one that is planned to be permanent even after the >>> move to blink_wpt_tests is done. >>> >>> On Fri, May 27, 2022 at 6:10 PM Domenic Denicola <dom...@chromium.org> >>> wrote: >>> >>>> +1. This was really unpleasantly surprising. When I first saw the >>>> original blink-dev email, I thought "generic baselines" meant something >>>> like "non-web platform test baselines", not "WPT expectation files that are >>>> platform-agnostic". >>>> >>>> In addition to the context-switching cost, it's just much harder to >>>> navigate between tests and their expectations, which is something I do >>>> quite often. E.g., they are no longer grouped together in code reviews, >>>> since their file paths are lexicographically far away from each other. And, >>>> as someone maintaining and reviewing several WPT directories, moving these >>>> crucial files out of the directories I commonly work in (and have metadata >>>> marking me as the point-of-contact for) into separate directories dilutes >>>> the cohesiveness of my projects. >>>> >>>> >>>> On Fri, May 27, 2022 at 12:03 PM Dominic Farolino <d...@chromium.org> >>>> wrote: >>>> >>>>> I write a lot of web platform tests as a Web Platform engineer; >>>>> recently I wrote one in external/wpt/ >>>>> <https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/external/wpt/> >>>>> (the external web platform tests directory), and was shocked to find the >>>>> eradication of `-expected.txt` files. I placed my expectations file next >>>>> to >>>>> the source file as we've done for many years, and found that my test was >>>>> "failing" because the test runner couldn't find my test expectations file. >>>>> >>>>> I dug deeper and found https://crrev.com/c/3603221 which was >>>>> responsible for moving more than 21,000 *platform-agnostic* test >>>>> expectations files away from their source files and into >>>>> web_tests/platform/generic >>>>> <https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/platform/generic/> >>>>> directory. I found more discussion in this email thread >>>>> <https://groups.google.com/a/chromium.org/g/blink-dev/c/0WmmgEkqdOo> which >>>>> I missed because blink-dev emails do not go directly in my inbox. >>>>> >>>>> I must say I find this change extraordinarily inconvenient as a Web >>>>> Platform engineer, and I want to push back against this. A minority of web >>>>> platform tests have platform-specific failures, which justifies the need >>>>> for *some* platform-specific test expectations directories, but I >>>>> believe a huge majority have generic baselines that are wildly convenient >>>>> to have right next to the actual tests themselves. Putting them in a >>>>> separate directory means I and others have to open a separate browser tab >>>>> to view how many expectations there are for a given directory, and >>>>> requires >>>>> a lot of unnecessary context switching. It is particularly confusing for >>>>> clusters of tests whose names are all *very* similar and vary by only >>>>> a few numbers or suffixes—this increases the cost of the context >>>>> switching. >>>>> >>>>> Furthermore, it renders tons of directories absolutely useless! All >>>>> ~150 directories in web_tests/virtual >>>>> <https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/virtual/> >>>>> (for >>>>> VirtualTestSuites) are just empty directories with README files—these used >>>>> to house virtualtest-specific expectations. So now for fenced frames (the >>>>> project I'm working on right now), we have the following test directories: >>>>> >>>>> - web_tests/wpt_internal/fenced_frame/ >>>>> >>>>> <https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/wpt_internal/fenced_frame/> >>>>> - web_tests/virtual/fenced-frame-mparch/ >>>>> >>>>> <https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/virtual/fenced-frame-mparch/> >>>>> - web_tests/virtual/fenced-frame-shadow-dom/ >>>>> >>>>> <https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/virtual/fenced-frame-shadow-dom/> >>>>> - web_tests/platform/generic/wpt_internal/fenced_frame/ >>>>> >>>>> <https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/platform/generic/wpt_internal/fenced_frame/> >>>>> - web_tests/platform/generic/virtual/fenced-frame-mparch/ >>>>> >>>>> <https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/platform/generic/virtual/fenced-frame-mparch/> >>>>> - web_tests/platform/generic/virtual/fenced-frame-shadow-dom/ >>>>> >>>>> <https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/platform/generic/virtual/fenced-frame-shadow-dom/> >>>>> - + platform-specific directories, which are relatively rare for us >>>>> >>>>> This is so weird! Regardless of whether or not there are plans to >>>>> clean this up, I can't see the upsides. The ousting of platform-agnostic >>>>> expectations is only an inconvenience for WP engineers, while there >>>>> might be some test-infra conveniences around BUILD.gn dependencies >>>>> <https://bugs.chromium.org/p/chromium/issues/detail?id=1299834#:~:text=This%20way%20all%20the%20baselines%20will%20be%20in%20platform%20directory%2C%20make%20it%20a%20little%20bit%20easier%20to%20specify%20dependency%20in%20BUILD.gn.> >>>>> (maybe?). >>>>> In any case, I am hard pressed to find justification in this move, and >>>>> would love to see if we can reconsider this. >>>>> >>>>> Thoughts? >>>>> >>>>> Dom >>>>> >>>>> -- >>>>> 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/CAP-uykAN06y5o-WYznnicvm1YREbSsLbs6dM57LtL4vCWB%3Duzw%40mail.gmail.com >>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAP-uykAN06y5o-WYznnicvm1YREbSsLbs6dM57LtL4vCWB%3Duzw%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>>> . >>>>> >>>> -- >>>> 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/CAM0wra9jXeotZVYNKBMmW90x36%2BdOCqcqfZ-ZpPW0qJVUBptbQ%40mail.gmail.com >>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAM0wra9jXeotZVYNKBMmW90x36%2BdOCqcqfZ-ZpPW0qJVUBptbQ%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>> . >>>> >>> -- 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+unsubscr...@chromium.org. To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CADXrSiqTkd1ZmZRbhHpO0FnyJKMhRwEtBX7_0QX9ZQ054LKigA%40mail.gmail.com.
