Web Platform Tests: Gathering feedback on import notifications

75 views
Skip to first unread message

Robert Ma

unread,
Sep 5, 2017, 3:24:09 PM9/5/17
to blink-dev, ecosystem-infra
Hello Blink developers,

I'm Robert Ma from the Ecosystem Infra team, and we maintain Chromium infrastructures related to Web Platform Tests (wpt), including the synchronization between our copy (LayoutTests/external/wpt) and github.com/w3c/web-platform-tests. I'm writing this email to gather feedback on the email notifications sent by our importer bot (i.e. GitHub->Chromium).

Some of you may have received emails from "Blink WPT Bot" via Gerrit, with titles like "Import wpt@[git-sha]". These are the email notifications we send during automatic imports with the intention of notifying owners of upstream changes (new tests, breakage, etc.). Owners of the test directories affected by an import are CC'ed in the import CL. There is also an excerpt in the CL description listing the affected directories and their owners (example CL: https://crrev.com/c/638954).

If you've seen these emails, we would like to ask you:
  • Do you find these emails useful? Or too spamy?
  • If you find them spamy, what is/are the main reason(s)? e.g.
    • Frequency is too high. (Would it be better if we only send emails for successful imports, and only once per CL after it lands? Currently, we add owners to the CC list early in the process, so you also get trybot/CQ updates etc.)
    • The value of information is too low, even if the improvements above were in place. (Perhaps your team doesn't currently follow up on upstream wpt changes?)
  • Do you wish to have some other information that's not currently present?
All comments and suggestions are much appreciated. As an "infra" team, we definitely want to make developers happy and we are open to all options, including turning off the notification if most people find it not useful.

Thank you!


Best,
Robert

Domenic Denicola

unread,
Sep 5, 2017, 3:39:41 PM9/5/17
to Robert Ma, blink-dev, ri...@chromium.org, ecosystem-infra
+ricea@ and I help maintain the streams/ directory. I think we generally find them a bit spammy (but not horribly so). Some main reasons are:

* Mostly they just notify us of changes that we ourselves made upstream via GitHub. So this doesn't help us much.
* What I really care about is whether any of the imported tests are marked as failing. Figuring this out takes extra work to dig into the CL and find -expected.txt files or changes.
* Most of the emails are bots talking to themselves and approving each other. Generally you get 6 emails for any given import: 3 from "Blink WPT Bot (Gerrit)", 2 from "Commit Bot (Gerrit)", and then one from "Quinten Yearsley (Gerrit)". Only the first of those emails has useful content.

So indeed something like an email summarizing successful imports, especially if it highlighted newly-failing tests with easily clickable links to the -expected.txt files, would be better.

Philip Jägenstedt

unread,
Sep 6, 2017, 7:58:37 AM9/6/17
to Domenic Denicola, Robert Ma, blink-dev, ri...@chromium.org, ecosystem-infra
I get some of these notifications as well, and I always archive them without reading. That's because unless I were to process all of them carefully, there's little chance that the ones I look at are the most important test failures to investigate.

What I'd find useful is an easy way to list all problems relating to LayoutTests/external/wpt/foo, combining TestExpectations, W3CImportExpectations and -expected.txt files. On top of that, maybe something like the Blink bug status mails with links to click through to details.

At some point there may be parts of the test suite that are so green that reacting to each new failure makes sense, and then perhaps opting in to notifications like these would be useful.

--
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 post to this group, send email to ecosyst...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/ecosystem-infra/BLUPR0501MB8523FC3DA834E5C59765206DF960%40BLUPR0501MB852.namprd05.prod.outlook.com.

Eddy Mead

unread,
Sep 7, 2017, 12:26:23 AM9/7/17
to Philip Jägenstedt, Domenic Denicola, Robert Ma, blink-dev, ri...@chromium.org, ecosystem-infra
We get these notifications to our list, style-dev@.  I don't think anyone on my team actually reads them, because a wide range of stuff goes to style-dev (often things we didn't work on), so it's mostly just noise to us.

Some feedback summarised from a quick team discussion:
- We don't send any other CLs to style-dev@, so it feels particularly spammy when robot CLs cc us
- We don't understand why we're getting cc'd on these CLs 
- They don't require any action from us
- We wouldn't know what to do if the import failed

On Wed, Sep 6, 2017 at 9:58 PM, 'Philip Jägenstedt' via blink-dev <blin...@chromium.org> wrote:
I get some of these notifications as well, and I always archive them without reading. That's because unless I were to process all of them carefully, there's little chance that the ones I look at are the most important test failures to investigate.

What I'd find useful is an easy way to list all problems relating to LayoutTests/external/wpt/foo, combining TestExpectations, W3CImportExpectations and -expected.txt files. On top of that, maybe something like the Blink bug status mails with links to click through to details.

At some point there may be parts of the test suite that are so green that reacting to each new failure makes sense, and then perhaps opting in to notifications like these would be useful.

On Tue, Sep 5, 2017 at 9:39 PM Domenic Denicola <d...@domenic.me> wrote:
+ricea@ and I help maintain the streams/ directory. I think we generally find them a bit spammy (but not horribly so). Some main reasons are:

* Mostly they just notify us of changes that we ourselves made upstream via GitHub. So this doesn't help us much.
* What I really care about is whether any of the imported tests are marked as failing. Figuring this out takes extra work to dig into the CL and find -expected.txt files or changes.
* Most of the emails are bots talking to themselves and approving each other. Generally you get 6 emails for any given import: 3 from "Blink WPT Bot (Gerrit)", 2 from "Commit Bot (Gerrit)", and then one from "Quinten Yearsley (Gerrit)". Only the first of those emails has useful content.

So indeed something like an email summarizing successful imports, especially if it highlighted newly-failing tests with easily clickable links to the -expected.txt files, would be better.

--
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-infra+unsubscribe@chromium.org.

--
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/CAARdPYf0nO-pQhVYPzPMyrquppaamzU-MFZP0jf%2B%3Du1o515UxA%40mail.gmail.com.

Philip Jägenstedt

unread,
Sep 12, 2017, 11:28:21 AM9/12/17
to Eddy Mead, Domenic Denicola, Robert Ma, blink-dev, ri...@chromium.org, ecosystem-infra
Thanks Eddy, I think that all matches what I suspected. An OWNERS tweak I did ought to have stopped the emails, but the question remains how to deal with this going forward.

On "We wouldn't know what to do if the import failed", the idea is definitely that no team should ever have to care, and that imports should happen at least every day. (For now, maybe getting down to <1 hour would unlock some additional workflows.)

An as of yet unsolved question is what workflow would make sense for new test failures. The basic ingredient is "make people notice", but how and how frequently I don't know.

Robert, in light of what Domenic and Eddy have said, what do you think we should do in the short term?

On Thu, Sep 7, 2017 at 12:26 AM Eddy Mead <me...@chromium.org> wrote:
We get these notifications to our list, style-dev@.  I don't think anyone on my team actually reads them, because a wide range of stuff goes to style-dev (often things we didn't work on), so it's mostly just noise to us.

Some feedback summarised from a quick team discussion:
- We don't send any other CLs to style-dev@, so it feels particularly spammy when robot CLs cc us
- We don't understand why we're getting cc'd on these CLs 
- They don't require any action from us
- We wouldn't know what to do if the import failed

On Wed, Sep 6, 2017 at 9:58 PM, 'Philip Jägenstedt' via blink-dev <blin...@chromium.org> wrote:
I get some of these notifications as well, and I always archive them without reading. That's because unless I were to process all of them carefully, there's little chance that the ones I look at are the most important test failures to investigate.

What I'd find useful is an easy way to list all problems relating to LayoutTests/external/wpt/foo, combining TestExpectations, W3CImportExpectations and -expected.txt files. On top of that, maybe something like the Blink bug status mails with links to click through to details.

At some point there may be parts of the test suite that are so green that reacting to each new failure makes sense, and then perhaps opting in to notifications like these would be useful.

On Tue, Sep 5, 2017 at 9:39 PM Domenic Denicola <d...@domenic.me> wrote:
+ricea@ and I help maintain the streams/ directory. I think we generally find them a bit spammy (but not horribly so). Some main reasons are:

* Mostly they just notify us of changes that we ourselves made upstream via GitHub. So this doesn't help us much.
* What I really care about is whether any of the imported tests are marked as failing. Figuring this out takes extra work to dig into the CL and find -expected.txt files or changes.
* Most of the emails are bots talking to themselves and approving each other. Generally you get 6 emails for any given import: 3 from "Blink WPT Bot (Gerrit)", 2 from "Commit Bot (Gerrit)", and then one from "Quinten Yearsley (Gerrit)". Only the first of those emails has useful content.

So indeed something like an email summarizing successful imports, especially if it highlighted newly-failing tests with easily clickable links to the -expected.txt files, would be better.

--
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.

Robert Ma

unread,
Sep 14, 2017, 2:30:19 PM9/14/17
to blink-dev, ecosyst...@chromium.org
Hello everyone,

First of all, thanks so much for your valuable inputs!

We had a team discussion yesterday with the outcome documented in detail at https://crbug.com/765334. The summary is:

* We will turn off the current notification via Gerrit this week. (If you have have different opinions or have better ideas for immediate action, feel free to reply or contact me directly.)
* We would like to explore some kind of auto bug filing for new failures/regressions introduced by imported changes. It will be configured per top-level (spec) directory, and will be OPT-IN.

Now, we'd like to find some test owners/teams who are interested in the auto bug filing feature so that we can discuss some more concrete requirements before actually designing and implementing.

Let's suppose the tests in the WPT directory you own is complete and stable enough that you want to keep track of regression, what kind of reports would you like? e.g. one new bug per new failing test file, or one new reply in a per-directory tracking bug? And what information you'd like to know in the report?

Thank you!

Eddy Mead

unread,
Sep 14, 2017, 8:50:08 PM9/14/17
to Robert Ma, blink-dev, ecosystem-infra
Hi Robert,

I think a new reply in a per-directory bug when failing tests are imported would be the right balance for style. It would be great if the comment included both the name(s) of the new failing test(s), and also a list of all failing tests. I can imagine a situation where the bug is quite long lived, and some tests get fixed along the way, so automatically keeping track of the set of failing tests would be helpful.

Cheers,
Eddy

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.



--
Regards,


Edwina Mead
Software Engineer
Google Australia

Victor Costan

unread,
Sep 15, 2017, 4:38:04 AM9/15/17
to Robert Ma, blink-dev, ecosystem-infra
I'd prefer a new bug per test. The failing bugs should be added as blocking bugs to a per-directory tracking bug. I prefer individual bugs to replies so it's easier to track and prioritize the effort of fixing each test. I'd be willing to own and review such bugs for IndexedDB.

Thank you,
    Victor

Adam Rice

unread,
Sep 15, 2017, 7:02:54 AM9/15/17
to Victor Costan, Robert Ma, blink-dev, ecosystem-infra
On 15 September 2017 at 17:37, Victor Costan <pwn...@chromium.org> wrote:
I'd prefer a new bug per test. The failing bugs should be added as blocking bugs to a per-directory tracking bug. I prefer individual bugs to replies so it's easier to track and prioritize the effort of fixing each test. I'd be willing to own and review such bugs for IndexedDB.

Me too. There isn't necessarily a common cause between tests that break at the same time. They may just have been updated in a batch. Since some degree of triage is necessary anyway, it's easier if the failures are broken down by test.

In some directories we have known deviations from the standard (url) or the wpt tests are in a bad state (websockets), and there would be a ridiculous number of bugs filed, but in practice we'd never turn on notifications for those directories anyway.

Philip Jägenstedt

unread,
Sep 18, 2017, 7:00:03 PM9/18/17
to Adam Rice, Victor Costan, Robert Ma, blink-dev, ecosystem-infra
I think the current options are:
  • One tracking bug for any failure anywhere (what we current have, not actionable)
  • One tracking bug per directory (would start out OK, become less and less actionable)
  • One new bug per import per directory with new failures.
  • One new bug per import per file with new failures.
  • One new bug per import per new failing test.
One thing to keep in mind is that because import is running very smoothly now, the changes to any given test are very likely to come from a single wpt PR. So with a bit of heuristics I think we could also have:
  • One new bug per imported wpt commit that caused failures.
Eventually I think we will want to be able to link an individual failing test in wpt.fyi to a crbug, bugzilla, etc., and the best granularity of bugs would be one that exactly matches the groups of test that will be fixed together. I think that in the first list of options per-directory is the best approximation, but the final per-commit option is also likely to be close.

Thoughts?

Robert Ma

unread,
Sep 18, 2017, 9:39:03 PM9/18/17
to blink-dev, Adam Rice, Victor Costan, Philip Jägenstedt, ecosystem-infra, jsb...@chromium.org
I had some discussion with Joshua (jsbell@) and just realized it was off the thread, so I'm pasting some of his insightful feedbacks here:

>>> from jsbell@

(1) ability to subscribe the public team list (storage-dev@); i.e. notifications would not contain any google-sensitive data. 
(2) either one subscription for multiple directories (e.g. one message covering deltas across FileAPI/, IndexedDB/, service-workers/cache-storage/, storage/, etc) *OR* if multiple subscriptions are necessary, ensure that the directory is clearly called out in the subject/body
(3) batched mails on newly imported/exported tests. e.g. One per day maximum, showing deltas
(4) batched mails on pass/failure, e.g. one per day, showing deltas
(5) everything linkified to wpt.fyi and w3c-test.org so it can be verified
(6) links to the PRs for additions/removals. If there's email or a bug reporting that there's a new test, removed test, or a test status change as a result of an import or an export it would be ideal if it linked to the github pull request (PR) that altered the tests. Example: if a test starts failing, it would be useful to click a link in the report to see that the change was c/o a Mozilla engineer's modification upstreamed by their infra. 
(7) wpt.fyi coverage results for other browsers, if possible

I could imagine 3+4 being merged, depending on presentation. e.g. (without thinking too much)

IndexedDB/
new tests:
    a.html    pass: 30        fail: 2
    b.html    pass: 32
deleted tests:
    c.html
existing tests:
    d.html    pass: 10 (-2)  fail: 2 (+2)
    e.html    pass: 6 (+5)   fail: 0 (-5)

This tells me there's new coverage that we're doing well on (yay!), something was removed (is this good? don't know - better check the PR), and for existing tests we're suddenly doing better on one and worse on another - better follow up!

-1 on "per-directory tracking bug". I've found these long-lived bugs to not get the right attention.

But otherwise... I think new bugs for failures is probably the minimum. Email with deltas (positive and negative) would be useful but not as critical.

<<< EOF

Christian Biesinger

unread,
Nov 8, 2017, 8:17:26 PM11/8/17
to blink-dev, ecosyst...@chromium.org
Catching up after my leave...

I am certainly very interested in notifications when tests in directories I care about fail. I actually liked the gerrit emails but it sounds like I was the only one.

I'd be happy with either:
- for each import that adds a new failure, file one bug per directory with test failures
- for each import that adds a new failure, file one bug per file with test failures

What's the current status of this?

Christian

Robert Ma

unread,
Nov 8, 2017, 8:27:29 PM11/8/17
to Christian Biesinger, blink-dev, ecosystem-infra
Hi Christian,

Thanks for the input. Your preferred option is actually the compromise that we'd like to first trial after team discussions.

This is one of my primary Q4 OKRs. Sorry not too much progress has been made due to my vacation and travel, but I plan to send out a design doc soon.

Best,
Robert

--
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 post to this group, send email to ecosyst...@chromium.org.

Philip Jägenstedt

unread,
Nov 27, 2017, 5:22:19 AM11/27/17
to Robert Ma, Christian Biesinger, blink-dev, ecosystem-infra

Christian Biesinger

unread,
Nov 29, 2017, 5:41:21 PM11/29/17
to Philip Jägenstedt, Robert Ma, blink-dev, ecosystem-infra
Thank you! That looks great.

Christian

Robert Ma

unread,
Nov 29, 2017, 6:10:21 PM11/29/17
to blink-dev, ecosystem-infra
Hi everyone,


I have made some adjustments based on initial reviews since Philip sent the link two days ago. Now the design has been stabilized, but suggestions and comments are still welcome!

(BCC'ing people who've contributed thoughts and ideas in this thread in case the previous email missed your inbox.)

Thank you!
Reply all
Reply to author
Forward
0 new messages