Bug 238832

Summary: Fix expected, actual links for variant-based imported wpt tests
Product: WebKit Reporter: pascoe <pascoe>
Component: WebKit WebsiteAssignee: Philippe Normand <philn>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, ews-watchlist, glenn, jbedard, jond, pascoe, simon.fraser, webkit-bug-importer, zimmermann
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=238862
https://bugs.webkit.org/show_bug.cgi?id=248745
Bug Depends on: 231544, 239561    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch none

pascoe@apple.com
Reported 2022-04-05 13:53:41 PDT
Appears these aren't working on ex: https://build.webkit.org/results/GTK-Linux-64-bit-Release-Tests/r292391%20(6950)/results.html
Attachments
Patch (8.73 KB, patch)
2022-04-05 15:10 PDT, pascoe@apple.com 		no flags
DetailsFormatted DiffDiff
Patch (10.61 KB, patch)
2022-04-06 09:09 PDT, pascoe@apple.com 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2022-04-05 13:57:52 PDT
<rdar://problem/91313891>
pascoe@apple.com
Comment 2 2022-04-05 14:40:24 PDT
I wiped out the associated change in results.html from https://bugs.webkit.org/show_bug.cgi?id=231544 when preparing it for a patch. Will add in this bug.
pascoe@apple.com
Comment 3 2022-04-05 15:10:01 PDT
Created attachment 456758 [details] Patch
pascoe@apple.com
Comment 4 2022-04-06 09:09:35 PDT
Created attachment 456825 [details] Patch
Brent Fulgham
Comment 5 2022-04-06 09:45:14 PDT
Comment on attachment 456825 [details] Patch r=me. Thanks for putting together a more complete/future-proof fix!
EWS
Comment 6 2022-04-06 14:59:03 PDT
Committed r292508 (249351@main): <https://commits.webkit.org/249351@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 456825 [details].
Nikolas Zimmermann
Comment 7 2022-04-12 02:24:45 PDT
I can no longer open differences/expected/actual files for SVG files, if they fail. Since this is the only code that touches the "output_filename" function in a while, it must be guilty for this.
Nikolas Zimmermann
Comment 8 2022-04-12 02:37:03 PDT
(In reply to Nikolas Zimmermann from comment #7) > I can no longer open differences/expected/actual files for SVG files, if > they fail. > Since this is the only code that touches the "output_filename" function in a > while, it must be guilty for this. Some context: if e.g. svg/W3C-SVG-1.1/animate-elem-02-t.svg fails, the generated results.html now contains links to "file:///Users/nzimmermann/Software/GitRepositories/WebKitVanilla/WebKitBuild/Release/layout-test-results/svg/W3C-SVG-1.1/animate-elem-02-t.svg-diffs.html" The ".svg" is superfluous and thus the link points to nirvana now.
Nikolas Zimmermann
Comment 9 2022-04-12 02:56:38 PDT
(In reply to Nikolas Zimmermann from comment #8) > (In reply to Nikolas Zimmermann from comment #7) > > I can no longer open differences/expected/actual files for SVG files, if > > they fail. > > Since this is the only code that touches the "output_filename" function in a > > while, it must be guilty for this. > > Some context: if e.g. svg/W3C-SVG-1.1/animate-elem-02-t.svg fails, the > generated results.html now contains links to > "file:///Users/nzimmermann/Software/GitRepositories/WebKitVanilla/ > WebKitBuild/Release/layout-test-results/svg/W3C-SVG-1.1/animate-elem-02-t. > svg-diffs.html" > > The ".svg" is superfluous and thus the link points to nirvana now. The JS code in LayoutTests/fast/harness/results.html implements the 'static testPrefix' method, which contains following check: else if (Utils.splitExtension(parts[0])[1].length > 5) { Utils.splitExtension(parts[0]) returns "["svg/W3C-SVG-1", "1/shapes-rect-01-t"] in my case -- it treats the dot in the folder name as file extension. The implementation of Utils.splitExtension() needs to be reworked: static splitExtension(testName) { let index = testName.lastIndexOf('.'); if (index == -1) { return [testName, '']; } return [testName.substring(0, index), testName.substring(index + 1)]; } Ok, I'll revert it locally for now, so that I can proceed to do what I actually wanted :-) Feel free to proceed with a fix rather than a roll-out. I'm happy to cross check your patch.
Nikolas Zimmermann
Comment 10 2022-04-20 13:08:01 PDT
This is hitting other folks as well, heard the question now two times already in Igaia, why results.html pages are broken.
Philippe Normand
Comment 11 2022-11-04 11:36:22 PDT
Pull request: https://github.com/WebKit/WebKit/pull/6143
EWS
Comment 12 2022-11-15 11:22:28 PST
Committed 256703@main (3be2c0d6c2ab): <https://commits.webkit.org/256703@main> Reviewed commits have been landed. Closing PR #6143 and removing active labels.
Simon Fraser (smfr)
Comment 13 2022-12-04 10:15:25 PST
This is still broken: bug 248745
Note You need to log in before you can comment on or make changes to this bug.