Bug 246323 – [Stress Test EWS] failing to find modified layout tests for pull requests

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Fujii Hironori]

[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.

[Radar WebKit Bug Importer]

<rdar://problem/101034678>

[Ryan Haddad]

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

[Fujii Hironori]

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

[Aakash Jain]

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.

[Aakash Jain]

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.

[Ryan Haddad]

*** Bug 249719 has been marked as a duplicate of this bug. ***

[Jonathan Bedard]

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.

[Jonathan Bedard]

Pull request: https://github.com/WebKit/WebKit/pull/8744

[EWS]

Committed 259088@main (692cd4308e63): <https://commits.webkit.org/259088@main>

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

[Fujii Hironori]

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

[Aakash Jain]

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.

[Jonathan Bedard]

Pull request: https://github.com/WebKit/WebKit/pull/13119

[Jonathan Bedard]

https://github.com/WebKit/WebKit/pull/13119 is a bit speculative, because our staging instance is not having these same problems.

[Jonathan Bedard]

(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.

[EWS]

Committed 263376@main (47b0e0b8761c): <https://commits.webkit.org/263376@main>

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