0

Set wpt-importer notification from opt-in to opt-out

Bug: 1454853
Change-Id: I59e1c4927de0f79e4581d0cf593a1217eb038817
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4803074
Reviewed-by: Weizhong Xia <weizhong@google.com>
Reviewed-by: Jonathan Lee <jonathanjlee@google.com>
Commit-Queue: An Sung <ansung@google.com>
Cr-Commit-Position: refs/heads/main@{#1187581}
This commit is contained in:
An Sung
2023-08-24 00:05:08 +00:00
committed by Chromium LUCI CQ
parent d82a9eed31
commit 183d2ac410
5 changed files with 50 additions and 19 deletions

@ -208,22 +208,20 @@ For maintainers:
### New failure notifications
Test owners can elect to have the importer automatically file bugs against a
component when imported changes introduce failures. This includes new tests that
fail in Chromium, as well as new failures introduced to an existing test. To
opt-in to this functionality, create an `DIR_METADATA` file in the appropriate
`external/wpt/` subdirectory that contains at least `wpt.notify` and
`monorail.component` fields. For example, `external/wpt/css/css-grid/DIR_METADATA`
looks like:
The importer automatically file bugs against a component when imported changes
introduce failures as long as test owners did not choose to opt-out the failure
notification mechanism. This includes new tests that fail in Chromium, as well
as new failures introduced to an existing test. Test owners are encouraged to
create an `DIR_METADATA` file in the appropriate `external/wpt/` subdirectory
that contains at least `monorail.component` fields, which will be used by the
importer to file the bugs.
For example, `external/wpt/css/css-grid/DIR_METADATA` looks like:
```
monorail {
component: "Blink>Layout>Grid"
}
team_email: "layout-dev@chromium.org"
wpt {
notify: YES
}
```
When tests under `external/wpt/css/css-grid/` newly fail in a WPT import, the
@ -236,8 +234,20 @@ The importer will also copy `layout-dev@chromium.org` (the `team_email`) and any
Failing tests are grouped according to the most specific `DIR_METADATA` that
they roll up to.
Note that we are considering making the notifications opt-out instead of
opt-in: see https://crbug.com/1454853
To opt-out of this notification, add `wpt.notify` field set to `NO` to the
corresponding `DIR_METADATA`.
For example, the following `DIR_METADATA` will suppress notification from tests
under the located directory:
```
monorail {
component: "Blink>Layout>Grid"
}
team_email: "layout-dev@chromium.org"
wpt {
notify: NO
}
```
### Skipped tests (and how to re-enable them)

@ -186,10 +186,8 @@ class DirectoryOwnersExtractor:
return None
# The value of `notify` is one of ['TRINARY_UNSPECIFIED', 'YES', 'NO'].
# Assume that users opt out by default; return True only when notify is
# 'YES'.
#
# TODO(crbug.com/1454853): Consider opt-in by default.
# Assume that users opt in by default; return False only when notify is
# 'NO'.
return WPTDirMetadata(data.get('teamEmail'),
data.get('monorail', {}).get('component'),
data.get('wpt', {}).get('notify') == 'YES')
data.get('wpt', {}).get('notify') != 'NO')

@ -245,7 +245,7 @@ class DirectoryOwnersExtractorTest(unittest.TestCase):
wpt_dir_metadata = extractor.read_dir_metadata(MOCK_WEB_TESTS + 'a/b')
self.assertIsNone(wpt_dir_metadata.team_email)
self.assertFalse(wpt_dir_metadata.should_notify)
self.assertTrue(wpt_dir_metadata.should_notify)
self.assertIsNone(wpt_dir_metadata.component)
def test_read_dir_all_fields(self):
@ -266,6 +266,18 @@ class DirectoryOwnersExtractorTest(unittest.TestCase):
self.host.executive = MockExecutive(output=data)
extractor = DirectoryOwnersExtractor(self.host)
wpt_dir_metadata = extractor.read_dir_metadata(MOCK_WEB_TESTS + 'a/b')
self.assertEqual(wpt_dir_metadata.team_email, 'bar')
self.assertTrue(wpt_dir_metadata.should_notify)
self.assertEqual(wpt_dir_metadata.component, 'foo')
def test_read_dir_disable_wpt(self):
data = (
'{"dirs":{"third_party/blink/web_tests/a/b":{"monorail":'
'{"component":"foo"},"teamEmail":"bar","wpt":{"notify":"NO"}}}}')
self.host.executive = MockExecutive(output=data)
extractor = DirectoryOwnersExtractor(self.host)
wpt_dir_metadata = extractor.read_dir_metadata(MOCK_WEB_TESTS + 'a/b')
self.assertEqual(wpt_dir_metadata.team_email, 'bar')
self.assertFalse(wpt_dir_metadata.should_notify)

@ -350,8 +350,15 @@ class ImportNotifier:
links_list = '\n[0]: https://chromium.googlesource.com/chromium/src/+/HEAD/docs/testing/web_test_expectations.md\n'
dir_metadata_path = self.host.filesystem.join(
directory, "DIR_METADATA")
epilogue = (
'\nTo opt out of WPT import notifications for this component, '
'add "wpt { notify: NO }" to "%s".' % dir_metadata_path)
description = (prologue + failure_list + expectations_statement +
range_statement + commit_list + links_list)
range_statement + commit_list + links_list +
epilogue)
bug = MonorailIssue.new_chromium_issue(summary,
description,

@ -502,6 +502,10 @@ class ImportNotifierTest(unittest.TestCase):
)
self.assertIn('crbug.com/12345 external/wpt/foo/baz.html [ Fail ]',
bugs[0].body['description'].splitlines())
self.assertIn(
'To opt out of WPT import notifications for this component, '
'add "wpt { notify: NO }" to "external/wpt/foo/DIR_METADATA".',
bugs[0].body['description'].splitlines())
def test_file_bug_without_owners(self):
"""A bug should be filed, even without OWNERS next to DIR_METADATA."""