204368 2019-11-19 12:09:11 -0800 EWS should use results database instead of doing clean-tree runs 2023-01-30 13:25:45 -0800 1 1 1 Unclassified WebKit Tools / Tests Other Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=224289 https://bugs.webkit.org/show_bug.cgi?id=248466 https://bugs.webkit.org/show_bug.cgi?id=248970 https://bugs.webkit.org/show_bug.cgi?id=251388 InRadar P2 Normal --- 248037 1 aakash_jain aakash_jain aakash_jain ahmad.saleem792 ap jbedard ryanhaddad webkit-bug-importer oldest_to_newest 1591641 0 aakash_jain 2019-11-19 12:09:11 -0800 EWS bots do clean-tree run when the test fails, in order to confirm if the test-failure is pre-existing or introduced by the patch. A lot of time, test-failure information is already available in results database (through build.webkit.org bots). We should try to get the clean-tree failure info from results database using its API, and avoid doing clean-tree runs. This would save a lot of time on EWS bots. 1591804 1 ap 2019-11-19 17:43:25 -0800 How can we confirm that this will be sufficient? It feels like EWS-only failures are somewhat common. I think that doing this in addition to retries is better than "instead" as the bug suggests. We can still retry with a clean tree if the failures are unknown. 1592031 2 jbedard 2019-11-20 09:45:44 -0800 (In reply to Alexey Proskuryakov from comment #1) > How can we confirm that this will be sufficient? It feels like EWS-only > failures are somewhat common. > > I think that doing this in addition to retries is better than "instead" as > the bug suggests. We can still retry with a clean tree if the failures are > unknown. We could do the retry if the failures are unknown, yes. And we might start that way. I think the retry is generally a bad idea, though. It means that in the case where an engineer did introduce a regression, we are going to be taking significantly longer to actually report that regression than in the successful case. Given the longer turn around for layout tests, I think this pretty significantly reduces the usefulness of EWS. We're essentially delaying results in the exact circumstances those results would be most impactful. I suppose we make the bubble orange on retry, but I feel like we might be training folks to ignore orange bubbles given how flakey things are at the moment. 1592049 3 ap 2019-11-20 10:31:45 -0800 If we don't retry, then we will have a lot of false positives, training engineers to ignore EWS altogether, not just in orange state. 1592072 4 383973 jbedard 2019-11-20 11:08:15 -0800 Created attachment 383973 Patch 1592074 5 383973 jbedard 2019-11-20 11:14:39 -0800 Comment on attachment 383973 Patch This patch should have been uploaded to https://bugs.webkit.org/show_bug.cgi?id=204409. 1592075 6 jbedard 2019-11-20 11:25:44 -0800 (In reply to Alexey Proskuryakov from comment #3) > If we don't retry, then we will have a lot of false positives, training > engineers to ignore EWS altogether, not just in orange state. I'm not entirely convinced that we need retries to get rid of false positives. There isn't a good reason that clean tree runs in EWS should differ from trunk results at all, and if we're cross-referencing trunk results, we could even expand the set of commits we're looking at to find flakey tests. This approach hinges on us quickly fixing things like https://bugs.webkit.org/show_bug.cgi?id=204409, but I really do think that things like that are bugs, not expected behavior. 1592168 7 ap 2019-11-20 15:23:45 -0800 One big reason is that EWS is different hardware. We definitely have quite a few tests that are only flaky or failing on EWS. 1595807 8 aakash_jain 2019-12-05 08:24:24 -0800 Seems like one thing we agree on is that, if the test(s) which fails in EWS is also marked as failing in results database, then we can skip the retry/clean-tree-run. If the test failure is new (not visible in results database), then we can continue with existing retry/clean-tree-run logic. This would still be a significant efficiency improvement, given that most of the times test failures seen in EWS are also seen in build.webkit.org 1595823 9 ap 2019-12-05 08:58:33 -0800 Correct. I think that we should use a fairly strict definition of "failing in results database', something like the test having failed within 24 hours, and maybe even require the same OS/flavor. 1792393 10 webkit-bug-importer 2021-09-10 09:27:43 -0700 <rdar://problem/82975679> 1911372 11 ryanhaddad 2022-11-09 14:45:47 -0800 *** Bug 247700 has been marked as a duplicate of this bug. *** 1914548 12 aakash_jain 2022-11-24 13:44:14 -0800 This was earlier blocked by results database not supporting git-hash or commit-identifier, and then due to not having appropriate api endpoint which ews could talk to. Both these issues were resolved. Implementing this as discussed above. 1914554 13 aakash_jain 2022-11-24 13:56:56 -0800 Pull request: https://github.com/apple/WebKit/pull/223 1914556 14 aakash_jain 2022-11-24 13:59:31 -0800 Pull request: https://github.com/WebKit/WebKit/pull/6792 1915114 15 ews-feeder 2022-11-28 14:44:34 -0800 Committed 257090@main (b9824fc290ac): <https://commits.webkit.org/257090@main> Reviewed commits have been landed. Closing PR #6792 and removing active labels. 383973 2019-11-20 11:08:15 -0800 2019-11-20 11:14:39 -0800 Patch bug-204368-20191120110815.patch text/plain 4972 jbedard SW5kZXg6IFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBUb29scy9DaGFuZ2VMb2cJKHJl dmlzaW9uIDI1MjY5NikKKysrIFRvb2xzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwz ICsxLDE5IEBACisyMDE5LTExLTIwICBKb25hdGhhbiBCZWRhcmQgIDxqYmVkYXJkQGFwcGxlLmNv bT4KKworICAgICAgICBFV1Mgc2hvdWxkIHVzZSByZXN1bHRzIGRhdGFiYXNlIGluc3RlYWQgb2Yg ZG9pbmcgY2xlYW4tdHJlZSBydW5zCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3No b3dfYnVnLmNnaT9pZD0yMDQzNjgKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICAqIEJ1aWxkU2xhdmVTdXBwb3J0L2V3cy1idWlsZC9zdGVwcy5weToKKyAg ICAgICAgKFJ1bldlYktpdFBlcmxUZXN0cy5ldmFsdWF0ZUNvbW1hbmQpOiBBZGQga2lsbE9sZFBy b2Nlc3NlcyBiZWZvcmUgcnVubmluZyB0ZXN0IHN1aXRlLgorICAgICAgICAoUnVuSmF2YVNjcmlw dENvcmVUZXN0cy5ldmFsdWF0ZUNvbW1hbmQpOiBEaXR0by4KKyAgICAgICAgKFJlUnVuSmF2YVNj cmlwdENvcmVUZXN0cy5ldmFsdWF0ZUNvbW1hbmQpOiBEaXR0by4KKyAgICAgICAgKFJ1bldlYktp dFRlc3RzLmV2YWx1YXRlQ29tbWFuZCk6IERpdHRvLgorICAgICAgICAoUmVSdW5XZWJLaXRUZXN0 cy5ldmFsdWF0ZUNvbW1hbmQpOiBEaXR0by4KKyAgICAgICAgKFJ1bkFQSVRlc3RzLmV2YWx1YXRl Q29tbWFuZCk6IERpdHRvLgorICAgICAgICAoUmVSdW5BUElUZXN0cy5ldmFsdWF0ZUNvbW1hbmQp OiBEaXR0by4KKwogMjAxOS0xMS0yMCAgSm9uYXRoYW4gQmVkYXJkICA8amJlZGFyZEBhcHBsZS5j b20+CiAKICAgICAgICAga2lsbC1vbGQtcHJvY2Vzc2VzOiBMb2cgY3VycmVudGx5IHJ1bm5pbmcg cHJvY2Vzc2VzCkluZGV4OiBUb29scy9CdWlsZFNsYXZlU3VwcG9ydC9ld3MtYnVpbGQvc3RlcHMu cHkKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PQotLS0gVG9vbHMvQnVpbGRTbGF2ZVN1cHBvcnQvZXdzLWJ1aWxkL3N0ZXBz LnB5CShyZXZpc2lvbiAyNTI2OTMpCisrKyBUb29scy9CdWlsZFNsYXZlU3VwcG9ydC9ld3MtYnVp bGQvc3RlcHMucHkJKHdvcmtpbmcgY29weSkKQEAgLTYzMiw3ICs2MzIsNyBAQCBjbGFzcyBSdW5X ZWJLaXRQZXJsVGVzdHMoc2hlbGwuU2hlbGxDb21tCiAgICAgZGVmIGV2YWx1YXRlQ29tbWFuZChz ZWxmLCBjbWQpOgogICAgICAgICByYyA9IHNoZWxsLlNoZWxsQ29tbWFuZC5ldmFsdWF0ZUNvbW1h bmQoc2VsZiwgY21kKQogICAgICAgICBpZiByYyA9PSBGQUlMVVJFOgotICAgICAgICAgICAgc2Vs Zi5idWlsZC5hZGRTdGVwc0FmdGVyQ3VycmVudFN0ZXAoW1JlUnVuV2ViS2l0UGVybFRlc3RzKCld KQorICAgICAgICAgICAgc2VsZi5idWlsZC5hZGRTdGVwc0FmdGVyQ3VycmVudFN0ZXAoW0tpbGxP bGRQcm9jZXNzZXMoKSwgUmVSdW5XZWJLaXRQZXJsVGVzdHMoKV0pCiAgICAgICAgIHJldHVybiBy YwogCiAKQEAgLTk3NCw3ICs5NzQsNyBAQCBjbGFzcyBSdW5KYXZhU2NyaXB0Q29yZVRlc3RzKHNo ZWxsLlRlc3QpCiAgICAgICAgICAgICBzZWxmLmJ1aWxkLnJlc3VsdHMgPSBTVUNDRVNTCiAgICAg ICAgICAgICBzZWxmLmJ1aWxkLmJ1aWxkRmluaXNoZWQoW21lc3NhZ2VdLCBTVUNDRVNTKQogICAg ICAgICBlbHNlOgotICAgICAgICAgICAgc2VsZi5idWlsZC5hZGRTdGVwc0FmdGVyQ3VycmVudFN0 ZXAoW1ZhbGlkYXRlUGF0Y2godmVyaWZ5QnVnQ2xvc2VkPUZhbHNlLCBhZGRVUkxzPUZhbHNlKSwg UmVSdW5KYXZhU2NyaXB0Q29yZVRlc3RzKCldKQorICAgICAgICAgICAgc2VsZi5idWlsZC5hZGRT dGVwc0FmdGVyQ3VycmVudFN0ZXAoW1ZhbGlkYXRlUGF0Y2godmVyaWZ5QnVnQ2xvc2VkPUZhbHNl LCBhZGRVUkxzPUZhbHNlKSwgS2lsbE9sZFByb2Nlc3NlcygpLCBSZVJ1bkphdmFTY3JpcHRDb3Jl VGVzdHMoKV0pCiAgICAgICAgIHJldHVybiByYwogCiAgICAgZGVmIGNvbW1hbmRDb21wbGV0ZShz ZWxmLCBjbWQpOgpAQCAtMTA1MCw2ICsxMDUwLDcgQEAgY2xhc3MgUmVSdW5KYXZhU2NyaXB0Q29y ZVRlc3RzKFJ1bkphdmFTYwogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgVmFsaWRhdGVQYXRjaCh2ZXJpZnlCdWdDbG9zZWQ9RmFsc2UsIGFkZFVSTHM9RmFs c2UpLAogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgQ29t cGlsZUpTQ1RvVCgpLAogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgVmFsaWRhdGVQYXRjaCh2ZXJpZnlCdWdDbG9zZWQ9RmFsc2UsIGFkZFVSTHM9RmFsc2Up LAorICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgS2lsbE9s ZFByb2Nlc3NlcygpLAogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgUnVuSlNDVGVzdHNXaXRob3V0UGF0Y2goKSwKICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgIEFuYWx5emVKU0NUZXN0c1Jlc3VsdHMoKV0pCiAgICAg ICAgIHJldHVybiByYwpAQCAtMTI4OCw3ICsxMjg5LDE0IEBAIGNsYXNzIFJ1bldlYktpdFRlc3Rz KHNoZWxsLlRlc3QpOgogICAgICAgICAgICAgc2VsZi5idWlsZC5yZXN1bHRzID0gU1VDQ0VTUwog ICAgICAgICAgICAgc2VsZi5idWlsZC5idWlsZEZpbmlzaGVkKFttZXNzYWdlXSwgU1VDQ0VTUykK ICAgICAgICAgZWxzZToKLSAgICAgICAgICAgIHNlbGYuYnVpbGQuYWRkU3RlcHNBZnRlckN1cnJl bnRTdGVwKFtBcmNoaXZlVGVzdFJlc3VsdHMoKSwgVXBsb2FkVGVzdFJlc3VsdHMoKSwgRXh0cmFj dFRlc3RSZXN1bHRzKCksIFZhbGlkYXRlUGF0Y2godmVyaWZ5QnVnQ2xvc2VkPUZhbHNlLCBhZGRV UkxzPUZhbHNlKSwgUmVSdW5XZWJLaXRUZXN0cygpXSkKKyAgICAgICAgICAgIHNlbGYuYnVpbGQu YWRkU3RlcHNBZnRlckN1cnJlbnRTdGVwKFsKKyAgICAgICAgICAgICAgICBBcmNoaXZlVGVzdFJl c3VsdHMoKSwKKyAgICAgICAgICAgICAgICBVcGxvYWRUZXN0UmVzdWx0cygpLAorICAgICAgICAg ICAgICAgIEV4dHJhY3RUZXN0UmVzdWx0cygpLAorICAgICAgICAgICAgICAgIFZhbGlkYXRlUGF0 Y2godmVyaWZ5QnVnQ2xvc2VkPUZhbHNlLCBhZGRVUkxzPUZhbHNlKSwKKyAgICAgICAgICAgICAg ICBLaWxsT2xkUHJvY2Vzc2VzKCksCisgICAgICAgICAgICAgICAgUmVSdW5XZWJLaXRUZXN0cygp LAorICAgICAgICAgICAgXSkKICAgICAgICAgcmV0dXJuIHJjCiAKICAgICBkZWYgZ2V0UmVzdWx0 U3VtbWFyeShzZWxmKToKQEAgLTEzMjAsNiArMTMyOCw3IEBAIGNsYXNzIFJlUnVuV2ViS2l0VGVz dHMoUnVuV2ViS2l0VGVzdHMpOgogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgVmFsaWRhdGVQYXRjaCh2ZXJpZnlCdWdDbG9zZWQ9RmFsc2UsIGFkZFVSTHM9 RmFsc2UpLAogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg Q29tcGlsZVdlYktpdFRvVCgpLAogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgVmFsaWRhdGVQYXRjaCh2ZXJpZnlCdWdDbG9zZWQ9RmFsc2UsIGFkZFVSTHM9 RmFsc2UpLAorICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg S2lsbE9sZFByb2Nlc3NlcygpLAogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgUnVuV2ViS2l0VGVzdHNXaXRob3V0UGF0Y2goKV0pCiAgICAgICAgIHJldHVy biByYwogCkBAIC0xNjI2LDcgKzE2MzUsNyBAQCBjbGFzcyBSdW5BUElUZXN0cyhUZXN0V2l0aEZh aWx1cmVDb3VudCk6CiAgICAgICAgICAgICBzZWxmLmJ1aWxkLnJlc3VsdHMgPSBTVUNDRVNTCiAg ICAgICAgICAgICBzZWxmLmJ1aWxkLmJ1aWxkRmluaXNoZWQoW21lc3NhZ2VdLCBTVUNDRVNTKQog ICAgICAgICBlbHNlOgotICAgICAgICAgICAgc2VsZi5idWlsZC5hZGRTdGVwc0FmdGVyQ3VycmVu dFN0ZXAoW1ZhbGlkYXRlUGF0Y2godmVyaWZ5QnVnQ2xvc2VkPUZhbHNlLCBhZGRVUkxzPUZhbHNl KSwgUmVSdW5BUElUZXN0cygpXSkKKyAgICAgICAgICAgIHNlbGYuYnVpbGQuYWRkU3RlcHNBZnRl ckN1cnJlbnRTdGVwKFtWYWxpZGF0ZVBhdGNoKHZlcmlmeUJ1Z0Nsb3NlZD1GYWxzZSwgYWRkVVJM cz1GYWxzZSksIEtpbGxPbGRQcm9jZXNzZXMoKSwgUmVSdW5BUElUZXN0cygpXSkKICAgICAgICAg cmV0dXJuIHJjCiAKIApAQCAtMTY0Niw2ICsxNjU1LDcgQEAgY2xhc3MgUmVSdW5BUElUZXN0cyhS dW5BUElUZXN0cyk6CiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICBWYWxpZGF0ZVBhdGNoKHZlcmlmeUJ1Z0Nsb3NlZD1GYWxzZSwgYWRkVVJMcz1GYWxzZSks CiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBDb21waWxl V2ViS2l0VG9UKCksCiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICBWYWxpZGF0ZVBhdGNoKHZlcmlmeUJ1Z0Nsb3NlZD1GYWxzZSwgYWRkVVJMcz1GYWxzZSks CisgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBLaWxsT2xk UHJvY2Vzc2VzKCksCiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICBSdW5BUElUZXN0c1dpdGhvdXRQYXRjaCgpLAogICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgQW5hbHl6ZUFQSVRlc3RzUmVzdWx0cygpXSkKICAgICAg ICAgcmV0dXJuIHJjCg==