New issue
Advanced search Search tips

Issue 862161 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 862143



Sign in to add a comment

Prevent stable/webexposed from being disabled by TestExpectations

Project Member Reported by fs...@chromium.org, Jul 10

Issue description

As per:  http://crbug.com/862143 .

It seems that stable/webexposed has been disabled by sheriffs in the past, as in:
https://chromium-review.googlesource.com/c/chromium/src/+/1090837

We need to prevent this from ever happening.
My suggestion is that we hard-code stable/webexposed somewhere on our tooling to ignore any TestExpectation trying to remove it.
 
Comments from duplicate  bug 883453 :

virtual/stable/webexposed tests are used to help validate that the blink launch process is being followed when new APIs are shipped.  As such blink API owner review is required for changes to them: https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/virtual/stable/webexposed/OWNERS

However, there have been a couple cases where people have disabled some or all of these tests completely without (presumably) understanding the implications of doing so, and it's taken us weeks to notice that we no longer had protection against accidental API additions/removals (which have actually occurred in the past).

Perhaps we should add some infrastructure to the test runner to fail (or ignore) attempts to disable tests in virtual/stable/webexposed? Obviously we can't (and shouldn't try to) prevent someone determined to bypass our process (TBR is always an option). This is just to try to reduce the risk of accidents which I've now seen happen at least twice.
Cc: haraken@chromium.org cha...@chromium.org
 Issue 883453  has been merged into this issue.
Can we add a presubmit warning as well for all webexposed tests? I think someone should have good reason to disable even the normal webexposed/ tests

Sign in to add a comment