Bug 67239

Summary: [DRT] Normalize file:///tmp/LayoutTests in LayoutTestController::pathToLocalResource()
Product: WebKit Reporter: Jarred Nicholls <jarred>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: UNCONFIRMED ---    
Severity: Normal CC: darin, dpranke, eric, leandro, ossy, rakuco, rniwa
Priority: P2 Keywords: LayoutTestFailure
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 67258, 67259, 67254, 67255, 67256    
Bug Blocks: 64135    

Description Jarred Nicholls 2011-08-30 14:59:34 PDT
Umbrella ticket for each port to normalize file:///tmp/LayoutTests in pathToLocalResource.  Rather than have the test runner create a symlink, doing this will also allow Windows-based DRT to run successfully.
Comment 1 Jarred Nicholls 2011-08-30 21:13:04 PDT
DumpRenderTree and WebKitTestRunner should both be addressed at the same time.
Comment 2 Jarred Nicholls 2011-08-31 23:38:07 PDT
see Qt's DRT implementation: bug #67254
Related Chromium implementation code: http://pastie.org/2464106

This will fix tests that attempt to load local resources from /tmp/LayoutTests.  NRWT does not create a symlink, which wouldn't work on Windows anyways.  DRT/WTR should remap these URLs in pathToLocalResource.
Comment 3 Eric Seidel (no email) 2011-09-07 16:12:27 PDT
Historically we have intentionally isolated DumpRenderTree from knowing anything about filesystem layout.

It would be possible to do this, but we'd need to pass some additional information to DumpRenderTree on the command line.  I'm not sure that's a good idea, given the historical precedent of avoiding telling DRT about the filesystem.

The approach in bug 67254 is interesting (avoids passing this information to DRT), but also wrong, because it encodes information about WebKit build layout in the DRT executable.

I think the best approach would be to make it possible to generate these file urls from paths relative to the test files themselves.  I'm not sure why the test files don't just use relative paths as-is.
Comment 4 Jarred Nicholls 2011-09-07 16:49:53 PDT
(In reply to comment #3)
> Historically we have intentionally isolated DumpRenderTree from knowing anything about filesystem layout.
> 
> It would be possible to do this, but we'd need to pass some additional information to DumpRenderTree on the command line.  I'm not sure that's a good idea, given the historical precedent of avoiding telling DRT about the filesystem.
> 
> The approach in bug 67254 is interesting (avoids passing this information to DRT), but also wrong, because it encodes information about WebKit build layout in the DRT executable.

The "static" approach taken in 67254 was not ideal by any means.  Chromium also statically remaps to the root of the source tree, so it was an acceptable stop-gap.  A config file (json, etc.) of "paths to remap" would be the more robust approach going forward incase 1) more than just /tmp/LayoutTests needed to be remapped, and 2) the relative path between LayoutTests and DRT changed (which can obviously be different per port).

> 
> I think the best approach would be to make it possible to generate these file urls from paths relative to the test files themselves.  I'm not sure why the test files don't just use relative paths as-is.

There are some http/security tests that load on a remote http:// origin and try to load file:// assets.  If the pathing was relative then these tests would be compromised; an absolute file:// path is needed.  However, it'd be an interesting idea to translate a relative http:// path to the appropriate absolute file:// path in pathToLocalResource.  But the question of how that path is derived still remains.

I'd be for a configuration file to make the remapping more dynamic.  Any ideas on how to derive a path other than by the location of DRT?  It would only be useful for the remote => local security tests.  All local => local tests could simply be changed to relative paths (some might argue that's compromising some integrity of the test, but it's a bit of a stretch).
Comment 5 Jarred Nicholls 2011-09-07 17:17:21 PDT
The whole motive here is to avoid a symlink for Windows test runs.  But even if we say "who cares, create a symlink and run it in Cygwin", we still have to remap /tmp/LayoutTests to the Cygwin root, like the Win port does.  Maybe that's cleaner in the end and touches less ports (i.e. only those that *ought to* test on windows).  But the point is, pathToLocalResource still has a purpose in certain cases.

Only a handful of http tests need this capability, and all the bots that regressed can handle symlinks in some way and therefore can work around it by creating a symlink in NRWT.  It's a tiny issue that doesn't need a lot of attention other than getting the bots green asap.