Landing/exporting tests that are unstable until dev build picks them up

0 views
Skip to first unread message

Matthew Wolenetz

unread,
Oct 14, 2020, 5:06:17 PM10/14/20
to ecosyst...@chromium.org
In https://chromium-review.googlesource.com/c/chromium/src/+/2459431/15#message-9ff86d29ae50e026e178640ce8c336c7fb892be7, the change enables non-flaky tests for a new experimental Chromium feature (MSE-in-Workers). But the export bot gives warning because the tests, when run against a chromium dev build that doesn't contain the rest of the code which makes it non-flaky, are flaky.

What is the general recommendation for landing such a change?

Note that this is a feature for MSE v2 that is being incubated first in Chromium behind experimental flag, so it is not expected to pass (or be stable) on other browsers either in the short-term.

Thanks,
Matt

Robert Ma

unread,
Oct 14, 2020, 5:17:09 PM10/14/20
to wole...@chromium.org, ecosystem-infra
Hi Matt,

Thanks for the heads up! Sorry the delay of a deflaking change making its way to Dev is a known and annoying issue. For now, just give us a heads-up and go ahead landing the CL in such case, and a team member on rotation will force-merge the PR. If you forgot to reach out, someone on the team would reach out to you as well.

--
You received this message because you are subscribed to the Google Groups "ecosystem-infra" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ecosystem-inf...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/ecosystem-infra/CAADho6PBZJ%2BJ49kbLG7iA6J-_E7oybYi1j_1M8Lj2SHRk2YMMQ%40mail.gmail.com.

Stephen Mcgruer

unread,
Oct 14, 2020, 5:23:28 PM10/14/20
to Robert Ma, wole...@chromium.org, ecosystem-infra
Robert is absolutely correct (thanks Robert), but I'd like to add:

> Note that this is a feature for MSE v2 that is being incubated first in Chromium behind experimental flag, so it is not expected to pass (or be stable) on other browsers either in the short-term.

Tests that are unstable do cause pain for other browser vendors (when they import them and then have to discover that they are flaky), so if possible its best to write a test to fail consistently if support doesn't exist. (If there's an easy way to detect support, you can use assert_implements to quickly bail with a clear error message.)

If you want to just merge and move on, that is 100% fine! But if you're interested and could explain why your test flaky-fails when feature support isn't present, I'd love to work with you to figure out if we could make it deterministically fail instead :).

Matthew Wolenetz

unread,
Oct 14, 2020, 6:21:31 PM10/14/20
to Stephen Mcgruer, Robert Ma, ecosystem-infra
Sounds good to me. I may be able to fix one source of other-browser flakiness (to fail 100% if they don't support the feature, instead of sometimes passing when the feature's lack of support isn't quite detected just yet.) But that may come in later change.
The Chromium dev flakiness should be fixed by the patch, and I'll confirm in waterfall after it lands before I let team member know to force-merge the GH PR.

Thanks,
Matt

Matthew Wolenetz

unread,
Oct 14, 2020, 6:30:53 PM10/14/20
to Stephen Mcgruer, Robert Ma, ecosystem-infra, Will Cassella
cc'ing cas...@google.com, who is reviewing the change.

Matthew Wolenetz

unread,
Oct 20, 2020, 6:41:59 PM10/20/20
to Stephen Mcgruer, Robert Ma, ecosystem-infra, Will Cassella
OK the following three changes are related, and should eventually have their external/wpt/media-source/... portions land/be exported/force-merged into external wpt:
Note that #3 has not quite landed yet in Chromium.

1) https://chromium-review.googlesource.com/c/chromium/src/+/2459431 (landed in Chromium tree last Friday. Exhibits flakiness in the mediasource-worker-play-terminate-worker test on implementations which don't yet support the experimental feature in the rest of that CL.
2) https://chromium-review.googlesource.com/c/chromium/src/+/2486074 (landed in Chromium tree yesterday (Monday)). Contains a test comment update, but otherwise doesn't change external flakiness. *Does* fix a Chromium flaky crash of the mediasource-worker-play-terminate-worker test.
3) https://chromium-review.googlesource.com/c/chromium/src/+/2487834 (in code-review currently). Once it lands, it should (when applied over the top of (1) and (2)) fix even external/other implementation flakiness for the mediasource-worker-play-terminate-worker test.

Matt

Robert Ma

unread,
Oct 20, 2020, 6:45:03 PM10/20/20
to wole...@chromium.org, Stephen Mcgruer, ecosystem-infra, Will Cassella
Thanks, Matt!

We've manually merged (1) and (2) in WPT. (3) should get exported soon and if it indeed fixes the flakiness the export PR will be merged automatically without need to intervene once the Chromium CL lands.

Matthew Wolenetz

unread,
Oct 20, 2020, 6:50:51 PM10/20/20
to Robert Ma, Stephen Mcgruer, ecosystem-infra, Will Cassella
Awesome, thanks! 
Reply all
Reply to author
Forward
0 new messages