New issue
Advanced search Search tips

Issue 684026 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature



Sign in to add a comment

[W3C auto-import] Improve the directory-owner functionality of the auto-importer

Project Member Reported by qyears...@chromium.org, Jan 23 2017

Issue description

Right now, when doing an auto-import, the auto-import script looks at a file called directory_owners.json, which has data originally populated from a spreadsheet, which has a list of dicts like this:

    {
        "component": "",
        "other-contact": "",
        "team": "DOM",
        "directory": "external/wpt/custom-elements/custom-element-lifecycle/types-of-callbacks",
        "notification-email": "dom-dev@chromium.org"
    },

Note: An incorrect email entry in this file has caused the latest failure here (https://build.chromium.org/p/chromium.infra.cron/builders/w3c-test-autoroller/builds/7893). So the first thing to do is to fix that.

But upon inspection of the file I think that the more important problem is that this file contains almost none of the actual owners of imported directories as listed in W3CImportExpectations.

Relatedly: domenic@ noted last week how it's important for someone to look at new expectations/baselines created by the auto-importer; for this reason, we want to add test directories owners for modified tests to the TBR line, ideally with a note about why they're added and what to look at. (This could also be filed as a separate issue.)
 

Comment 1 by tkent@chromium.org, Jan 24 2017

Cc: tkent@chromium.org
We're adding COMPONENT and TEAM fields to OWNERS files. Managing such information in a different way would be maintenance burden.  The atuoimporter should use them if possible.

How about having a mirror directory structure for OWNERS files? For example,

LayoutTests/
  external/
    csswg-test/
    wpt/
  external_owners/
    csswg-test/
     css-display-3/
       OWNERS
    wpt/
      FileAPI/
        OWNERS

Auto-importer copies OWNERS file in external_owners/ to external/ on importing, and may skip importing directories without OWNERS.

 Issue 667952  is a tracking bug to build a single JSON file from COMPONENT and TEAM.

Aye, thanks for telling me about that --

For now, what I thinking was just to only have a mapping of top-level directories to owner email addresses, which could be extracted from W3CImportExpectations -- but this wouldn't yet include component and team.

Later, it's possible that we will want to auto-file bugs for failures when importing ( bug 676672 ); then, keeping track of components would be useful.
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4edc8aac446bd6bc319b9430ed1bed30391f1ff1

commit 4edc8aac446bd6bc319b9430ed1bed30391f1ff1
Author: qyearsley <qyearsley@chromium.org>
Date: Wed Jan 25 23:10:40 2017

Extract test directory owner email addresses from W3CImportExpectations.

The purpose of this CL is to improve the list of people to CC that the
w3c-test-autoroller generates -- previously the list was based on a file
directory_owners.json, which was generated once (from a spreadsheet)
and had only a couple of (mailing list) addresses. This CL would make
it so that directory owners are extracted from W3CImportExpectations.

Another alternate approach would be to use W3CImportExpectations to
generate a JSON file, and read that, but then that way we would have to
regularly update the JSON file.

BUG= 684026 

Review-Url: https://codereview.chromium.org/2649363003
Cr-Commit-Position: refs/heads/master@{#446157}

[modify] https://crrev.com/4edc8aac446bd6bc319b9430ed1bed30391f1ff1/third_party/WebKit/LayoutTests/W3CImportExpectations
[delete] https://crrev.com/b8cf426b59b931395bfe5114c7c6952473f210ce/third_party/WebKit/Tools/Scripts/generate-w3c-directory-owner-json
[modify] https://crrev.com/4edc8aac446bd6bc319b9430ed1bed30391f1ff1/third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py
[delete] https://crrev.com/b8cf426b59b931395bfe5114c7c6952473f210ce/third_party/WebKit/Tools/Scripts/webkitpy/w3c/directory_owners.json
[add] https://crrev.com/4edc8aac446bd6bc319b9430ed1bed30391f1ff1/third_party/WebKit/Tools/Scripts/webkitpy/w3c/directory_owners_extractor.py
[add] https://crrev.com/4edc8aac446bd6bc319b9430ed1bed30391f1ff1/third_party/WebKit/Tools/Scripts/webkitpy/w3c/directory_owners_extractor_unittest.py

Status: Fixed (was: Assigned)
Note, this should now be "done", but not yet confirmed. Will re-open if I observe problems later.
Project Member

Comment 6 by bugdroid1@chromium.org, Feb 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4f0dd06e26f1813daf5b99191d437995e082f20b

commit 4f0dd06e26f1813daf5b99191d437995e082f20b
Author: qyearsley <qyearsley@chromium.org>
Date: Sat Feb 25 00:30:35 2017

Change directory owner listing to list joint owners together.

Before this CL, directory owners are listed in import CL descriptions,
but joint owners of directories are split up, e.g.:

domenic@chromium.org:
  external/wpt/streams
foolip@chromium.org:
  external/wpt/webvtt
maksim.sisov@intel.com:
  external/wpt/webvtt
nhiroki@chromium.org:
  external/wpt/workers
ricea@chromium.org:
  external/wpt/streams

This CL would make it so joint owners are listed together, e.g.:

domenic@chromium.org, ricea@chromium.org:
  external/wpt/streams
foolip@chromium.org, maksim.sisov@intel.com:
  external/wpt/webvtt
nhiroki@chromium.org:
  external/wpt/workers

BUG= 684026 

Review-Url: https://codereview.chromium.org/2716083002
Cr-Commit-Position: refs/heads/master@{#453017}

[modify] https://crrev.com/4f0dd06e26f1813daf5b99191d437995e082f20b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/directory_owners_extractor.py
[modify] https://crrev.com/4f0dd06e26f1813daf5b99191d437995e082f20b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/directory_owners_extractor_unittest.py
[modify] https://crrev.com/4f0dd06e26f1813daf5b99191d437995e082f20b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer.py
[modify] https://crrev.com/4f0dd06e26f1813daf5b99191d437995e082f20b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer_unittest.py

Components: Blink>Infra>Ecosystem
Components: -Blink>Infra>Predictability

Sign in to add a comment