Propose to re-enable triggering linux_layout_tests_layout_ng for LayoutNG changes

[Koji Ishii]

Hi all,

Please disregard if you're not planning LayoutNG changes.

Emil prepared a LayoutNG bot that runs all tests with "--enable-blink-features=LayoutNG" in crbug.com/706183. This is great to capture possible regressions, but was too great that we made it opt-in.

We have then added 23k expectations to FlagExpectations that I'd like to propose us to trigger the bot by default again for LayoutNG changes.

What should I do if linux_layout_tests_layout_ng bot failed?

At this stage, it's still quite possible that your change is good but the bot is failing for different reasons. For example, LayoutNG often render -expected file incorrectly today, so if your change fix -expected file, it'll be a new failure.

My proposal at this point is you can either:

• Just remove the CQ_INCLUDE_TRYBOTS line from your commit message, or
• Update "LayoutTests/FlagExpectations/enable-blink-features=LayoutNG" file.

Meaning I'd like to say this bot is not mandatory to land, but I think it still gives us good indications how a change affects tests.

The bot fails without patch, slowing try-bots for my CL...

The FlagExpectations needs maintenance, since new tests are added everyday and many of them are likely to fail.

This should not prevent your CL to land, since such tests should fail without patch too, but you may see the bot is slow because of it.

I'm volunteering to update the file, at least once a week or so, but in case your CL is in trouble by the slowness for some reasons, I think it's ok to remove the CQ_INCLUDE_TRYBOTS line, since this bot is more about providing info to the CL authors. Reminding me to update the file is also great. Volunteering to update it is even greater.

Anything else?

The re-enabling CL is here, I hope to land it if no objections, but please give your thoughts to the CL or to this thread.

One more thing you could help

During the maintenance, I can add failures to FlagExpectations, but passes will only be marked as flaky. If you know you fixed some tests that are in FlagExpectations, great if you can remove those lines. That way, if other change is going to break your fix, the bot will find it. I'm thinking to remove failures automatically somehow, such as 5 passes in a row, but I have not got to there yet.

[Koji Ishii]

Given no responses so far and Gleb and Emil were good in the CL, I'm planning to enable this tomorrow. Please reply or leave comments in the CL if any.

[Koji Ishii]

I think we can enable this now. You may still encounter bots preventing your CL to land, sorry in the case, please remove CQ_INCLUDE_TRYBOTS to land and let me know by e-mail/IRC/hangout.

Longer version: Got comments to increase the failure threshold doesn't look straightforward and infra looks busy, I'm still waiting for their responses.

However, this week was good; only ~300 failures (threshold 5000) and ~50 crashes-or-timeouts (threshold 100,) so the bot was happy. Someone may add a new import to add a few hundreds of tests that all crashes LayoutNG, so I'll continue the discussion with infra.

But as long as the workaround is tiny enough and the info it gives us is useful, I think we can enable and adjust as needed.

Please reply to this thread, or to me, if you encounter any problems.

[Koji Ishii]

This is now enabled. I hope this is good for all of us.

I also started removing fixed tests by finding 20 or so consecutive passes, and 1300 tests were fixed in the last two weeks. Great our work!

[Ian Kilpatrick]

This is great! Is there an easy way to update the test expectations? Or just done manually for the moment?

--
You received this message because you are subscribed to the Google Groups "layout-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to layout-dev+unsubscribe@chromium.org.
To post to this group, send email to layou...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/layout-dev/CACQRE%2BQj9-7ew4bY7pYxk6c-6Bsu%2B_9j8eDe-LCOdCGercTu%3DQ%40mail.gmail.com.

[Quinten Yearsley]

AFAIK we have to manually make CLs to add lines to "LayoutTests/FlagExpectations/enable-blink-features=LayoutNG" for test that fail under LayoutNG at HEAD.

One way to do this would be to go to the results for a run (e.g. https://storage.googleapis.com/chromium-layout-test-archives/linux_layout_tests_layout_ng/168/layout-test-results/results.html), click "Flag" and copying/pasting them at the bottom of "LayoutTests/FlagExpectations/enable-blink-features=LayoutNG" with the appropriate expectations.

Also: does anybody think it would be very useful to have a continuous builder on chromium.fyi running layout tests with the LayoutNG flag?

[Koji Ishii]

I'm running my own script to do what Quinten said.

If you want to help it to be up-to-date, clone the repo, "npm install", and:

$ ng-bot 168

will download the 168 job result and update the expectations file. I'll run it once or twice a week, or when the bot get red.

Note, unless crbug.com/714203 is resolved somehow, the bot will fail with 100 crashes/timeouts. We will then need to run tests locally to get the test results json because the bot gives up running the rest of tests.