Changes to scroll-anchoring tests in initial landing were backed out
Categories
(Core :: Layout: Scrolling and Overflow, enhancement, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox67 | --- | fixed |
People
(Reporter: rhunt, Assigned: rhunt)
References
Details
Attachments
(1 file)
I'm a bit confused what happened here.
There were some changes made to the tests under 'css/scroll-anchoring/' in bug 1305957. The patches took a couple tries to stick and get merged to nightly.
It seems like somehow the changes to the tests were backed out at some point after they landed [1] [2]. The reason given appears to be the ASAN failures that got the patches backed out initially. But those failures were resolved before it was finally merged into central.
We should reland the changes to the tests.
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1305957#c47
[2] https://hg.mozilla.org/mozilla-central/rev/9eb38c69446baadce8ca7840373eda1f909ade0c
|
Assignee | |
Comment 1•5 years ago
|
||
James, was this an issue with wpt-sync or something I did or forgot to do?
I'm not super familiar with how we're suppose to manage changes to WPTs.
|
|
Assignee | |
Comment 2•5 years ago
|
||
These changes were reviewed, landed, and accidentally backed out in bug 1528107.
|
||
Comment 3•5 years ago
•
|
||
It looks to me like what happened here was:
(A) rhunt made his initial landing (bug 1305957 comment 33), which I think spawned this automatic WPT pull request:
https://github.com/web-platform-tests/wpt/pull/14812
(B) He was backed out for ASAN failures (bug 1305957 comment 35) and the backout spawned this automatic WPT pull request:
https://github.com/web-platform-tests/wpt/pull/14816
(C) He re-landed later that day (bug 1305957 comment 38), but as far as I can tell, no automatic WPT pull request was spawned for this change!
(D) Later on, we somehow misinterpreted the backout pull request as having been an "upstream" change to WPT that we needed to merge into mozilla-central, and so we did (in bug comment 47).
Both (C) and (D) seem to indicate errors in our WPT synchronization process.
|
|
||
Comment 4•5 years ago
|
||
After this has made it to central, we should uplift this to beta, too.
(No approval is needed for test-only changes like these ones -- i.e. you can land there with a=testonly, since they're not part of the code that we ship to users. You should be sure this sticks on central before uplifting, though.)
|
|
||
Comment 5•5 years ago
|
||
(Actually, never mind -- the problematic "bonus backout" didn't happen until after the most recent merge day [when beta took a snapshot of central]. So, beta still has the correct changes and doesn't need anything extra. Hooray!)
|
||
Comment 6•5 years ago
|
||
Yeah, something weird has happened here. I think dholbert's analysis is correct; presumably at step C the changes relanding looked like a noop on upstream because the original change had merged but the backout hadn't. That would result in no new PR being opened and no error, because we sometimes have a case where someone manually landed changes from upstream, which obviously shouldn't cause problems.
So I think the underlying issue here is timing/ordering. In order to processs the backout as a separate PR to the original push we must have not seen it until the latter merged, which means the latter had landed on central. So one thing that could help is to ensure that we always fully process integration branches before we process central. But there's a bigger question of why we got so far behind the pushes in the first place; it's like we're missing events or something. I'll investigate what's going on.
Pushed by rhunt@eqrion.net: https://hg.mozilla.org/integration/mozilla-inbound/rev/6fa064cb22ba Reland changes to scroll-anchoring tests. r=dholbert
|
||
Comment 8•5 years ago
|
||
| bugherder | ||
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/15439 for changes under testing/web-platform/tests
Upstream PR merged
Description
•