Bug 238832 - Fix expected, actual links for variant-based imported wpt tests
Summary: Fix expected, actual links for variant-based imported wpt tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Website (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Philippe Normand
URL:
Keywords: InRadar
Depends on: 231544 239561
Blocks:
  Show dependency treegraph
 
Reported: 2022-04-05 13:53 PDT by pascoe@apple.com
Modified: 2022-12-04 10:15 PST (History)
9 users (show)

See Also:


Attachments
Patch (8.73 KB, patch)
2022-04-05 15:10 PDT, pascoe@apple.com
no flags Details | Formatted Diff | Diff
Patch (10.61 KB, patch)
2022-04-06 09:09 PDT, pascoe@apple.com
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description pascoe@apple.com 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
Comment 1 Radar WebKit Bug Importer 2022-04-05 13:57:52 PDT
<rdar://problem/91313891>
Comment 2 pascoe@apple.com 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.
Comment 3 pascoe@apple.com 2022-04-05 15:10:01 PDT
Created attachment 456758 [details]
Patch
Comment 4 pascoe@apple.com 2022-04-06 09:09:35 PDT
Created attachment 456825 [details]
Patch
Comment 5 Brent Fulgham 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!
Comment 6 EWS 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].
Comment 7 Nikolas Zimmermann 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.
Comment 8 Nikolas Zimmermann 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.
Comment 9 Nikolas Zimmermann 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.
Comment 10 Nikolas Zimmermann 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.
Comment 11 Philippe Normand 2022-11-04 11:36:22 PDT
Pull request: https://github.com/WebKit/WebKit/pull/6143
Comment 12 EWS 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.
Comment 13 Simon Fraser (smfr) 2022-12-04 10:15:25 PST
This is still broken: bug 248745