search

catapult-project / catapult

Deprecated Catapult GitHub. Please instead use http://crbug.com "Speed>Benchmarks" component for bugs and https://chromium.googlesource.com/catapult for downloading and editing source code..
https://chromium.googlesource.com/catapult
BSD 3-Clause "New" or "Revised" License
1.93k stars 564 forks source link

Bisect bug duping should not merge bugs where multiple culprits are found #4357

Closed anniesullie closed 6 years ago

anniesullie commented 6 years ago

Example: https://bugs.chromium.org/p/chromium/issues/detail?id=822866#c23

Bisect found 3 different culprits on this bug. One of the culprits caused multiple regressions, and all the other regressions were merged into the bug.

Bisect should not automatically merge into bugs where multiple culprits have been found. @simonhatch can you take a look? @dave-2 FYI

wangxianzhu commented 6 years ago

I'm not sure if this is a result of bisect error or a manual error which led to the bisect behavior.

For crbug.com/822866, there are several regressions in different chromium ranges (some are non-overlapping) grouped together: 543233:543293 543197:543232 543233:543287 I wonder if it was like this when the bug was first filed, or some unrelated regressions were added into the bug later.

The first bisect job for a regression in the first range found culprit r543291, and assigned the bug to me (correct). When I examined the regressions, I found different patterns of the regressions so I scheduled more bisect jobs, which found different culprits: crbug.com/822866#c10: r543283 crbug.com/822866#c14: r543229 -> skia@8a3f55c659

Then I noticed that the regressions grouped together had non-overlapping ranges and ranges not including r543291, so I checked the regressions one by one, removed crbug.com/822866 from them, and filed some new bugs hoping they could be regrouped. After that, I closed crbug.com/822866 which didn't have any regression associated with it.

However, when the bisect jobs of the new bugs finished, all new bugs were merged back into crbug.com/822866.

I thought of the following:

anniesullie commented 6 years ago

Thanks so much for clarifying!

The system does give a warning when associating non-overlapping regression ranges.

The system may prompt "non-overlapping ranges" when a user tries to associate a regression to a bug with non-overlapping regression range.

We actually do give this warning; looks like it was ignored in this case.

If non-related regressions are grouped together, and bisect jobs find different culprits, the system may split the bug automatically or under manual intervention.

In the short term, I'm working on making a list of bugs with clear follow-up action items for sheriffs in #4312. I'll make sure to add this case there.

When regressions are removed from a bug, the system may disconnect the corresponding found culprit from the bug, in order to prevent new bisect jobs from merging the new bug back into the original bug.

Definitely. Simon, can you work on this part?

simonhatch commented 6 years ago

Thanks for clarifying what happened here, and yeah I can take this.

simonhatch commented 6 years ago

Landed code to prevent this, so hopefully won't happen again.