Bug 246323

Summary: [Stress Test EWS] failing to find modified layout tests for pull requests
Product: WebKit Reporter: Fujii Hironori <Hironori.Fujii>
Component: Tools / TestsAssignee: Jonathan Bedard <jbedard>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, ap, ddkilzer, jbedard, ryanhaddad, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=223890
https://bugs.webkit.org/show_bug.cgi?id=250714
https://bugs.webkit.org/show_bug.cgi?id=251157
Bug Depends on: 242735    
Bug Blocks: 239082    

Description Fujii Hironori 2022-10-10 23:11:56 PDT
[Stress Test EWS] failing to find modified layout tests for pull requests

Stress Test EWS is working fine for patch workflow, but for PR workflow.
Comment 1 Radar WebKit Bug Importer 2022-10-11 08:54:53 PDT
<rdar://problem/101034678>
Comment 2 Ryan Haddad 2022-10-11 16:30:13 PDT
https://github.com/WebKit/WebKit/pull/4783 is a PR that added/modified WPT tests, but it appears that the stress test bot skipped running tests https://ews-build.webkit.org/#/builders/62/builds/35773
Comment 3 Fujii Hironori 2022-12-06 12:41:56 PST
How's this going?

This PR added a new test, but mac-AS-debug-wk2 did nothing.
https://github.com/WebKit/WebKit/pull/7119

On the other hand, in the patch work flow, it works as expected.
https://bugs.webkit.org/show_bug.cgi?id=248812
Comment 4 Aakash Jain 2022-12-13 15:41:19 PST
Looking at https://ews-build.webkit.org/#/builders/62/builds/35773, it didn't have "modified_tests" build property, and find-modified-layout-tests step didn't log any output, it seems like self._get_patch() inside FindModifiedLayoutTests.start() didn't return the list of files from PR (https://github.com/WebKit/WebKit/blob/main/Tools/CISupport/ews-build/steps.py#L1249). 

_get_patch() (inside AnalyzeChange base class) was last changed in https://commits.webkit.org/246362@main to add support for PRs. Seems like it might not be working for PRs properly.
Comment 5 Aakash Jain 2022-12-13 16:59:45 PST
Just for reference, here is an example of successful patch build: https://ews-build.webkit.org/#/builders/62/builds/42495

It has 'modified_tests' build property set, and 'find-modified-layout-tests' step also logs the test names.
Comment 6 Ryan Haddad 2022-12-21 09:09:29 PST
*** Bug 249719 has been marked as a duplicate of this bug. ***
Comment 7 Jonathan Bedard 2023-01-17 10:26:55 PST
Part of our problem here is clearly https://bugs.webkit.org/show_bug.cgi?id=250714, although I'm not convinced that's our only problem. https://github.com/WebKit/WebKit/pull/4783 clearly had this problem, but https://github.com/WebKit/WebKit/pull/7119 doesn't, so there is something else going on.
Comment 8 Jonathan Bedard 2023-01-17 17:20:23 PST
Pull request: https://github.com/WebKit/WebKit/pull/8744
Comment 9 EWS 2023-01-19 09:29:03 PST
Committed 259088@main (692cd4308e63): <https://commits.webkit.org/259088@main>

Reviewed commits have been landed. Closing PR #8744 and removing active labels.
Comment 10 Fujii Hironori 2023-03-27 18:34:51 PDT
This bug doesn't seem to be fixed. Reopened.
The following jobs did nothing for pull requests that added/changed tests.

https://ews-build.webkit.org/#/builders/8/builds/215
https://ews-build.webkit.org/#/builders/8/builds/776
Comment 11 Aakash Jain 2023-04-24 05:28:34 PDT
We added more logging in 263193@main, which clearly indicates this issue. 

Almost every PR based build (e.g.: https://ews-build.webkit.org/#/builders/8/builds/3731), now shows the log "Unable to access the patch/PR content." in find-modified-layout-tests step.
Comment 12 Jonathan Bedard 2023-04-24 14:42:24 PDT
Pull request: https://github.com/WebKit/WebKit/pull/13119
Comment 13 Jonathan Bedard 2023-04-24 14:50:56 PDT
https://github.com/WebKit/WebKit/pull/13119 is a bit speculative, because our staging instance is not having these same problems.
Comment 14 Jonathan Bedard 2023-04-25 07:34:12 PDT
(In reply to Jonathan Bedard from comment #13)
> https://github.com/WebKit/WebKit/pull/13119 is a bit speculative, because
> our staging instance is not having these same problems.

Ok, https://github.com/WebKit/WebKit/pull/13119 is no longer speculative, I understand what's going on!

When buildbot triggers a build from another build, by default, it will re-compute the source stamp. For our EWS instance, this is always the wrong thing to do. EWS should use the source stamp from the triggering build, which will contain the list of modified files.
Comment 15 EWS 2023-04-25 09:44:44 PDT
Committed 263376@main (47b0e0b8761c): <https://commits.webkit.org/263376@main>

Reviewed commits have been landed. Closing PR #13119 and removing active labels.