I wouldn't particularly want to make this a standard step on the bots, because that's just not really how the code is structured.
We could potentially do something as part of the isolate step. The code's not really structured for that, either, but that would be less distasteful than adding a separate step. -- Dirk On Tue, Aug 17, 2021 at 11:47 AM Xianzhu Wang <[email protected]> wrote: > Can we make this a standalone step or run it in the "isolate test" step on > the bot? It's like a preparation before sharding blink_web_tests, to reduce > the duplicated work on the swarming bots. > > On Tue, Aug 17, 2021 at 11:17 AM Domenic Denicola <[email protected]> > wrote: > >> >> >> On Tue, Aug 17, 2021 at 2:08 PM 'Weizhong Xia' via blink-dev < >> [email protected]> wrote: >> >>> Running collect_web_tests on external/wpt is kind of slow, because we >>> need to generate wpt manifest first. Running that on other folders will be >>> fast. And I will take a look to see if there is room for improvement. >>> >>> One thing worth mentioning is that if the change in external/wpt is >>> meant to be exported to upstream >>> <https://github.com/web-platform-tests/wpt>, you can ignore the >>> presubmit check warning. (Should have mentioned this in the PSA). >>> >> >> Aren't all changes in external/wpt meant to be exported to upstream? From >> what I understand that's the purpose of the directory... >> >> >>> >>> On Tue, Aug 17, 2021 at 11:01 AM Dirk Pranke <[email protected]> wrote: >>> >>>> On Tue, Aug 17, 2021 at 10:48 AM Ian Kilpatrick < >>>> [email protected]> wrote: >>>> >>>>> So I was a little surprised by this last night. >>>>> >>>> >>>> Yeah, we probably should've had the discussion first, sorry :(. >>>> >>>> >>>>> >>>>> 1. This was relatively slow on my macbook, running locally takes ~40s. >>>>> From just now: >>>>> >>>>> ikilpatrick-macbookpro:src ikilpatrick$ touch >>>>> third_party/blink/web_tests/external/wpt/css/css-tables/test.html >>>>> >>>>> ikilpatrick-macbookpro:src ikilpatrick$ time >>>>> third_party/blink/tools/collect_web_tests.py external/wpt >>>>> >>>>> >>>>> real 0m40.492s >>>>> >>>>> user 0m23.007s >>>>> >>>>> sys 0m41.715s >>>>> >>>>> 2. I had a bunch of uncommitted tests in various wpt directories that >>>>> I needed to move out to somewhere else (rather than the script just adding >>>>> committed tests). >>>>> >>>> >>>> One other concern is that we might end up with a lot of merge conflicts >>>> in a single file, and so we might want to shard by the top level >>>> directories or something ... >>>> >>>> (I'd also note that uploading patches is already relatively slow, do we >>>>> have metrics for how long developers are waiting for upload scripts to >>>>> run? >>>>> - I suspect this is due to WPT presubmit scripts but unsure) >>>>> >>>> >>>> The upload checks are just generally crazy slow these days. I'm not >>>> sure if we have collected metrics for them, but things are an order of >>>> magnitude slower than I'd like them to be. >>>> >>>> But that's partially a separate topic (apart from this check, of >>>> course). >>>> >>>> -- Dirk >>>> >>>> >>>>> Ian >>>>> >>>>> >>>>> >>>>> >>>>> On Tue, Aug 17, 2021 at 10:20 AM 'Dirk Pranke' via blink-dev < >>>>> [email protected]> wrote: >>>>> >>>>>> >>>>>> >>>>>> On Tue, Aug 17, 2021 at 10:08 AM 'Weizhong Xia' via blink-dev < >>>>>> [email protected]> wrote: >>>>>> >>>>>>> tl;dr: We introduced a mechanism that pre-collects test cases to >>>>>>> //third_party/blink/web_tests/AllTestsByDirectories.json, and uses a >>>>>>> presubmit check to ensure this file is up to date. You can stop here if >>>>>>> you >>>>>>> don't write web test cases. >>>>>>> >>>>>>> Dear blink-devs, >>>>>>> >>>>>>> You may have noticed that when you try to upload a CL, you are >>>>>>> prompted to update AllTestsByDirectories.json if your CL changes web >>>>>>> test >>>>>>> cases. Yes, today we landed a CL >>>>>>> <https://chromium-review.googlesource.com/c/chromium/src/+/3067372> >>>>>>> that pre-collects test cases and saves that to the json file. The >>>>>>> reason to >>>>>>> do this is that rwt tries to collect tests on every swarming bots, and >>>>>>> that >>>>>>> takes about 30 seconds (the best case when running on SSD). This is not >>>>>>> a >>>>>>> small amount of time as we are targeting a 10-min CQ. After this CL >>>>>>> lands, >>>>>>> collecting tests now takes less than 2 seconds for all platforms. >>>>>>> >>>>>>> Impact to you: if your CL adds/deletes/renames web test cases, you >>>>>>> will be prompted. Follow the instructions there should be enough. But if >>>>>>> you still have an issue, (even though we have tested many different >>>>>>> scenarios we can think about), feel free to ping/email me. >>>>>>> >>>>>>> There is no change to how you run rwt locally. We added a new switch >>>>>>> to tell the swarming bots to load tests from the json file. This switch >>>>>>> is >>>>>>> turned off by default. Developers usually don't need to turn this on, >>>>>>> unless all of your tests are saved to the json file and you are running >>>>>>> a >>>>>>> full test so want to save about 30 seconds. >>>>>>> >>>>>> >>>>>> FWIW, I think this is an important and probably worthwhile change >>>>>> (disclaimer: I did review it and was involved in the design). The number >>>>>> of >>>>>> tests we have now is substantial and the startup delay is a significant >>>>>> cost. We need to be looking into ways to reduce this. >>>>>> >>>>>> I think it's possible we should turn this on by default and/or do >>>>>> some hooking to make things aware of when you've added new tests (e.g., >>>>>> via >>>>>> git status) to try and make this a little more seamless, but I also think >>>>>> landing it off by default was a good first step. >>>>>> >>>>>> Some may ask if we can just generate this at compile time. >>>>>> Unfortunately, no, we can't, because there's no way to keep it correctly >>>>>> up-to-date when you do add tests after having generated the file at least >>>>>> once [ we asked this ourselves and actually tried this approach and broke >>>>>> deterministic builds :( ]. >>>>>> >>>>>> I'm happy to hear other thoughts. Perhaps there are other ways we can >>>>>> optimize things to be fast enough ... >>>>>> >>>>>> -- Dirk >>>>>> >>>>>> >>>>>>> >>>>>>> Thanks for your time! >>>>>>> >>>>>>> Cheers, Weizhong >>>>>>> >>>>>>> -- >>>>>>> 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 [email protected]. >>>>>>> To view this discussion on the web visit >>>>>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CADXrSiprRWy_UCC0w2oohbSGGTbeP%3DdkyYnBOx7j8Y-abDSe2A%40mail.gmail.com >>>>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CADXrSiprRWy_UCC0w2oohbSGGTbeP%3DdkyYnBOx7j8Y-abDSe2A%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 [email protected]. >>>>>> To view this discussion on the web visit >>>>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAEoffTAc389E6wteVXUAuKX8ZBFWBa2UEMwAbd9oCxJ9pxNCKQ%40mail.gmail.com >>>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAEoffTAc389E6wteVXUAuKX8ZBFWBa2UEMwAbd9oCxJ9pxNCKQ%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 [email protected]. >>> To view this discussion on the web visit >>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CADXrSipBCHvjwf-AA1DGHUVjVzKQka5bBvTBnSRx5sEUpnJN_Q%40mail.gmail.com >>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CADXrSipBCHvjwf-AA1DGHUVjVzKQka5bBvTBnSRx5sEUpnJN_Q%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 [email protected]. >> To view this discussion on the web visit >> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAM0wra9B-aZTjoovP1WGDL80od64CtEL30jM8ch-2Pm_cd9Log%40mail.gmail.com >> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAM0wra9B-aZTjoovP1WGDL80od64CtEL30jM8ch-2Pm_cd9Log%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 [email protected]. To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAEoffTA0pg5V92k-6gOORv5J%3DOiNVEuDH6VyW2tKTKjdd2uK%2BA%40mail.gmail.com.
