Bug 248745
| Summary: | In results.html, links to diffs for tests with periods in the name fail | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> |
| Component: | Tools / Tests | Assignee: | Jonathan Bedard <jbedard> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | jbedard, pascoe, philn, 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=238832 | ||
Simon Fraser (smfr)
When a test like http/wpt/service-workers/fetch-service-worker-preload-download.https.html fails, the expected, actual, diff and pretty diff links in results. html are broken. For example, they link to:
file://null/Volumes/Data/Development/system/webkit/OpenSource/WebKitBuild/Debug/layout-test-results/http/wpt/service-workers/fetch-service-worker-preload-download.https.html-pretty-diff.html
instead of:
file://null/Volumes/Data/Development/system/webkit/OpenSource/WebKitBuild/Debug/layout-test-results/http/wpt/service-workers/fetch-service-worker-preload-download.https-pretty-diff.html
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Simon Fraser (smfr)
There was an attempt to fix in bug 238832. but that broke results for SVG which has periods in folder names.
Philippe Normand
The issue is that for this testName we end up in the "Temporary fix" case for which I have no context.
static testPrefix(testName)
{
let parts = Utils.splitExtension(testName);
let prefix = parts[0];
let remains = parts[2];
if (remains) {
if (remains.includes('?'))
prefix += '_' + remains.split('?')[1]
else if (remains.includes('#'))
prefix += '_' + remains.split('#')[1]
} else if (Utils.splitExtension(parts[0])[1].length > 5) {
// Temporary fix, also in Tools/Scripts/webkitpy/layout_tests/constrollers/test_result_writer.py, line 115.
// FIXME: Refactor to avoid confusing reference to both test and process names.
return testName;
}
return prefix;
}
Utils.splitExtension(parts[0])[1] == '.https' here, so we'd need an additional check, but as I lack the historical meaning of this code, I'm not sure how to fix this.
Simon Fraser (smfr)
It should be easy to handle both a period in the test file name, and a period in a directory name, which I think is all that's required to handle this?
Jonathan Bedard
Figured out the problem, results.html is using `Utils.splitExtension(parts[0])[1].length > 5` instead of `Utils.splitExtension(parts[0])[1].length - 1 > 5`, which is what the matching Python code does. I don't have a great answer for the "why", though. Probably worth the naive fix for know and sorting out the existing FixMe in the near future.
Radar WebKit Bug Importer
<rdar://problem/103041235>
Jonathan Bedard
Pull request: https://github.com/WebKit/WebKit/pull/7222
EWS
Committed 257527@main (fc8aae223895): <https://commits.webkit.org/257527@main>
Reviewed commits have been landed. Closing PR #7222 and removing active labels.