New issue
Advanced search Search tips

Issue 775439 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Jan 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

wpt-exporter is creating duplicate PRs

Project Member Reported by foolip@chromium.org, Oct 17 2017

Issue description

`git log --grep Change-Id --pretty=%s | sort | uniq -c | sort -n` shows 4 cases where we've had export PRs with the same first line.

The one that caught my attention today:

"Implemented: remove browsing context name on cross origin navigation":
https://github.com/w3c/web-platform-tests/pull/7072
https://github.com/w3c/web-platform-tests/pull/7783

These are created from the very same Chromium review, and have exactly the same changes. For some reason, exporter hasn't recognized that the change is already exported. Because of https://github.com/w3c/web-platform-tests/pull/7637, it was possible to export it again without conflicts, and possibly it will be exported again if the duplication is fixed.

The others:

"Move first set of variable fonts test to WPT":
https://github.com/w3c/web-platform-tests/pull/6350
https://github.com/w3c/web-platform-tests/pull/6525

A similar situation, already fixed in https://github.com/w3c/web-platform-tests/pull/6894

"Add tests for table/row/row-group directions":
https://github.com/w3c/web-platform-tests/pull/6321
https://github.com/w3c/web-platform-tests/pull/6344 (revert)
https://github.com/w3c/web-platform-tests/pull/6524 (reland)

This case is not a problem.

"[ServiceWorker] Fix wpt test fetch-canvas-tainting-iframe.html" isn't really a dupe, it's just the same title.
 

Comment 1 by foolip@chromium.org, Oct 17 2017

This is the job that created #7783:
https://build.chromium.org/p/chromium.infra.cron/builders/wpt-exporter/builds/44331

From the log:
2017-10-16 05:14:42,459 - Cloning GitHub w3c/web-platform-tests into /tmp/wpt
2017-10-16 05:15:12,694 - Found Gerrit in-flight CL: "[css-grid] Take into account min size constrains for stretching auto tracks" https://chromium-review.googlesource.com/720918
2017-10-16 05:15:22,179 - In-flight PR found: https://github.com/w3c/web-platform-tests/pull/7782
2017-10-16 05:15:22,179 - PR revision matches CL revision. Nothing to do here.
2017-10-16 05:15:22,179 - Found Gerrit in-flight CL: "[WIP] Support request bodies in ServiceWorkerSubresourceLoader." https://chromium-review.googlesource.com/708177
2017-10-16 05:15:22,180 - In-flight PR found: https://github.com/w3c/web-platform-tests/pull/7688
2017-10-16 05:15:22,180 - PR revision matches CL revision. Nothing to do here.
2017-10-16 05:15:22,180 - Found Gerrit in-flight CL: "Rename scroll-anchoring to css-scroll-anchoring in web-platform-tests." https://chromium-review.googlesource.com/719966
2017-10-16 05:15:22,180 - In-flight PR found: https://github.com/w3c/web-platform-tests/pull/7780
2017-10-16 05:15:22,180 - PR revision matches CL revision. Nothing to do here.
2017-10-16 05:15:22,180 - Found Gerrit in-flight CL: "CSS Motion Path: support <size> for ray paths" https://chromium-review.googlesource.com/670379
2017-10-16 05:15:22,181 - In-flight PR found: https://github.com/w3c/web-platform-tests/pull/7414
2017-10-16 05:15:22,181 - PR revision matches CL revision. Nothing to do here.
2017-10-16 05:15:59,483 - Found exportable Chromium commit: Implemented: remove browsing context name on cross origin navigation 57e5929e121f8f081a80a2faaf68b00552cf7e72
2017-10-16 05:15:59,554 - No PR found for Chromium commit. Creating...
2017-10-16 05:16:00,785 - Deleting old branch chromium-export-57e5929e12
2017-10-16 05:16:00,797 - Creating local branch chromium-export-57e5929e12
2017-10-16 05:16:01,263 - Author: "Andy Paicu <andypaicu@chromium.org>"

At that time the original PR had already been merged for a long time.

Comment 2 by foolip@chromium.org, Oct 17 2017

I can't find the job that created #7072, it's too long ago. It doesn't matter though, job 44331 should have found that exported commit regardless of how it ended up there.

Comment 3 by foolip@chromium.org, Oct 17 2017

I've deleted the security/ directory again in https://github.com/w3c/web-platform-tests/pull/7858.
Components: Blink>Infra>Ecosystem
I really struggle to come up with any sensible hypothesis.

Apparently, during wpt-import #44331, the importer somehow could not find the corresponding PR for 57e5929e121f8f081a80a2faaf68b00552cf7e72. The only reasons I can think of are:

1. The list of PRs was complete but the matching logic was wrong. -- Impossible. I tried reproducing the list of PRs at the time and the script successfully found the PR for that commit. And there has been no change in the relevant code recently.
2. The list of PRs was incomplete.
   2.1 The PR was out of history window. -- Impossible. We implemented pagination support long time ago. Since then, we can fetch up to 5000 PRs, and the current total number of PRs is only 700+.
   2.2 Network error. -- Impossible. We'd have raised a fatal exception: https://cs.chromium.org/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/w3c/wpt_github.py?sq=package:chromium&l=221
   2.3 Incomplete results (but indicated so in the response). -- Impossible. A fatal exception is raised: https://cs.chromium.org/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/w3c/wpt_github.py?sq=package:chromium&l=215
   2.4 Incomplete results (but not indicated). -- This seems like the only remaining possibility, though it violates what GitHub guarantees in their API docs.

I'm going to add some logging in the hope of finding something interesting.
Project Member

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

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

commit e89db9209913ee8d10c74b0a5cf287da50e21a7d
Author: Robert Ma <robertma@chromium.org>
Date: Wed Oct 25 20:57:46 2017

Log the number of PRs retrieved in all_pull_requests

Bug:  775439 
Change-Id: I6629736623e033fd22771d4a2802e12f04f64bd0
Reviewed-on: https://chromium-review.googlesource.com/738393
Commit-Queue: Robert Ma <robertma@chromium.org>
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511581}
[modify] https://crrev.com/e89db9209913ee8d10c74b0a5cf287da50e21a7d/third_party/WebKit/Tools/Scripts/webkitpy/w3c/wpt_github.py

Labels: -Pri-2 Pri-3
Yet another month has (almost) passed. And I haven't seen any new duplicate PRs. Downgrading to P3.
Status: WontFix (was: Assigned)
Closing as I haven't seen another case for a long time

Sign in to add a comment