webkit-patch rebaseline-cl gives incorrect baselines for Windows-only change |
||||
Issue descriptionA patch which only changes Windows baselines is producing incorrect baselines. It seems to have placed the Windows results in the common directory, and removed platform-specific results in the Windows, Mac and Linux directories. e.g., https://chromium-review.googlesource.com/c/514443/ webkit-patch rebaseline-cl was run on patch set 11, and the results checked into patch set 12. You can see that only the Windows trybots were red in patch set 11, but patch set 12 contains added files in the common directory, and removed platform-specific results.
,
Jun 22 2017
,
Jun 22 2017
In theory, if everything had worked right, rebaseline-cl should just download the new baselines, and then dedupe baselines only if they're exactly the same on all platforms - which I don't think should be the case here. Do you think you could you undo the last patchset (and get back into the state that things were at in patchset 11) and then try re-running the try jobs, and then re-run rebaseline-cl to see all of the messages that it prints? Note, I think that you can get your local state back to patchset 11 by doing: git fetch https://chromium.googlesource.com/chromium/src refs/changes/43/514443/11 && git checkout FETCH_HEAD If all fails (and there's an blocking bug in rebaseline-cl), it will be theoretically possible to use NeedsManualRebaseline, and rebaseline in a follow-up CL (but then you'll have a period of no coverage).
,
Jun 22 2017
Note, I'm also going to try to reproduce this here: https://chromium-review.googlesource.com/514443 Extra thoughts: It seems like here for some reason it might have used the windows baselines as cross-platform baselines (which is the behavior that it should have if you pass --fill-missing, but is not the right behavior here). In this case, rebaseline-cl should have observed that all other bots passed, only the windows bots failed, and only windows needed to be rebaselined...
,
Jun 22 2017
Update: So, after trying to reproduce this the first thing I notice is that it says it did not use the windows baselines to "fill in" other platforms baselines. I don't think I reproduced what happened originally, because I didn't get any new baselines in platform/mac/... I'm not sure now why this is. Note: If everything works correctly, I expect no new baselines should be added in LayoutTests/platform/mac. But, it could mean adding a new baseline under platform/linux because of an interesting quirk about the way the baselines are organized; the win baseline is a fallback for linux if there's no linux-specific baseline. So if the results were the same between win and linux before, and they become different, then a new linux-specific baseline is added. Anyway, after running rebaseline-cl, I uploaded another patchset at https://chromium-review.googlesource.com/544544 and started running the try jobs again.
,
Jul 9 2017
I never did reproduce the original issue here, and I think we should file a new issue and try to reproduce again if this comes up again, so I'll close this now. Meanwhile, for https://chromium-review.googlesource.com/c/514443/ (the CL mentioned in the original post), rebaseline-cl *should* work. The week before last I also worked out some of the other layout test result changes from that CL (flaky tests, failing ref tests, etc.) in https://chromium-review.googlesource.com/c/544544/.
,
Jul 12 2017
Just to clarify: the original problem was not that there were baselines added to Mac or Linux directories. The platform-specific baselines were *removed* from all those directories, and the new (Windows) baselines were put in the common, cross-platform directories (the ones which hold the test sources). This caused those tests to start failing on Mac and Linux, where they were passing before. You should be able to see this in patch set 12: https://chromium-review.googlesource.com/c/514443/12 I'll let Alexis re-open if this is still happening for him. |
||||
►
Sign in to add a comment |
||||
Comment 1 by dpranke@chromium.org
, Jun 22 2017