How to get notified when WPT auto-roller disables a test?

[Łukasz Anforowicz]

Hello,

I've added external/wpt/fetch/corb/img-html-correctly-labeled.sub.html layout test a few months ago and assumed that green CQ/waterfall means that the test is still passing.  It turns out that WPT import in r558677 on May 15th has seen that the test fails and added it to LayoutTests/TestExpectations.  This has been rather surprising (I've accidentally discovered that some CORB tests are failing by looking at https://master-dot-wptdashboard.appspot.com/results/fetch/corb - I expected all the tests to pass since CORB has been enabled by default in r553830 on April 25th).  I am glad that I didn't rely on WPT coverage alone to keep my feature healthy and had also some (possibly reduntant) end-to-end coverage through content_browsertests.

I guess one mistake I made was that I didn't add an explicit external/wpt/fetch/corb/OWNERS file (are these documented somewhere?  don't they conflict with code repositories used by other browsers?).  OTOH, I don't see external/wpt/fetch/OWNERS being CC-ed on https://chromium-review.googlesource.com/1059309 or notified in another way.  QUESTION: How can I make sure I am in the loop when WPT auto-roller decides to disable a test?

Thanks,

Lukasz

PS. TestExpectations are not the only way tests are silently disabled by WPT auto-roller.  Another one seems to be auto-generating -expected.txt files with failed asserts (e.g. see external/wpt/fetch/api/cors/cors-filtering.sub.any-expected.txt from r557294)

[robe...@google.com]

Hi Lukasz,

I added an "import notification" feature to WPT importer which does exactly what you want (automatically filing bugs to you for both new TestExpectations lines and new failing assertions/subtests in the -expected.txt baselines). See the PSA for details.

In your case, you can either:

* add "# WPT-NOTIFY: true" to the top of wpt/fetch/OWNERS (turns on notification for the whole wpt/fetch directory),

* or create a wpt/fetch/corb/OWNERS with "# COMPONENT", "# WPT-NOTIFY" and optionally your email if you only want to receive notifications for wpt/fetch/corb .

Your question also shows that this feature isn't very discoverable. In addition to the PSA, I should've added a section to docs/testing/web_platform_tests.md (or is there a more intuitive place that you'd expect to see the document for this feature?). There's also the option to make the notification opt-out instead of opt-in, as quite a few teams have enrolled and it's been working well so far.

Let me know what other improvements you'd like to see to make WPT more usable and reliable!

Best,

Robert

[Mounir Lamouri]

On Fri, 18 May 2018, at 22:31, robertma via blink-dev wrote:
> Hi Lukasz,
>
> I added an "import notification" feature to WPT importer which does
> exactly
> what you want (automatically filing bugs to you for both new
> TestExpectations lines and new failing assertions/subtests in the
> -expected.txt baselines). See the PSA

> <https://groups.google.com/a/chromium.org/d/topic/blink-dev/B-vHX5Bys24/discussion>

> for details.
>
> In your case, you can either:
> * add "# WPT-NOTIFY: true" to the top of wpt/fetch/OWNERS (turns on
> notification for the whole wpt/fetch directory),
> * or create a wpt/fetch/corb/OWNERS with "# COMPONENT", "# WPT-NOTIFY" and
> optionally your email if you only want to receive notifications for
> wpt/fetch/corb .
>
> Your question also shows that this feature isn't very discoverable. In
> addition to the PSA, I should've added a section to
> docs/testing/web_platform_tests.md (or is there a more intuitive place that
> you'd expect to see the document for this feature?). There's also the
> option to make the notification opt-out instead of opt-in, as quite a few
> teams have enrolled and it's been working well so far.

I was actually wondering about the same thing the other day and was hoping we had a notification system. Glad to know it's there but maybe it would indeed be worth having it opt-out? I believe developers would expect in general to be notified when a test is disabled, usually via a bug assigned to them on crbug, so we may want to make sure WPT keeps some sort of consistency.

Out of curiosity, what would be the use case to not receive notifications about disabled tests?

-- Mounir

[Philip Jägenstedt]

Initially we weren't sure if the notifications would be too spammy and people would turn them off after a week, so we didn't want to enable them everywhere at once. Since it seems to be working just fine, I think always filing bugs would be OK now. We could support "WPT-NOTIFY: false" as an opt-out, but it would be even less discoverable than the current opt-in, since initially it wouldn't be used anywhere at all.

[Stephen Mcgruer]

> it would be even less discoverable than the current opt-in

If you're going to file bugs automatically (woo! totally support this), then why not just put a brief paragraph about WPT-NOTIFY: false at the end? E.g.

"This bug was filed automatically due to a new WPT test failure for which you are marked an OWNER. If you don't want to receive these reports, please add WPT-NOTIFY: false to the relevant OWNERs file (bit.ly/wpt-notify-false)."

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAARdPYcbY6GuXDQxZ8WoSrg7UOFq_NiT%3DaPy5XP3RY4ueqZ4bA%40mail.gmail.com.

[Philip Jägenstedt]

On Mon, May 21, 2018 at 2:06 PM Stephen Mcgruer <smcg...@chromium.org> wrote:

> it would be even less discoverable than the current opt-in

If you're going to file bugs automatically (woo! totally support this), then why not just put a brief paragraph about WPT-NOTIFY: false at the end? E.g.

"This bug was filed automatically due to a new WPT test failure for which you are marked an OWNER. If you don't want to receive these reports, please add WPT-NOTIFY: false to the relevant OWNERs file (bit.ly/wpt-notify-false)."

Good suggestion, that would do it! Robert, can you file a bug about these suggested changes for people to follow?

[Weizhong Xia]

An update to this: we have made this CL that implement the ideas proposed above. All the folders are defaultly opt-in unless the owners choose to opt out. The bugs will be filed against blink>infra>ecosystem if no other components have been defined in the directory tree. You are encouraged to set the right component for your tests so that the importer knows where to file crbugs and we don't have to reassign a crbug later.

Thanks, Weizhong