Bug 67239 - [DRT] Normalize file:///tmp/LayoutTests in LayoutTestController::pathToLocalResource()
Summary: [DRT] Normalize file:///tmp/LayoutTests in LayoutTestController::pathToLocalR...
Status: UNCONFIRMED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: LayoutTestFailure
Depends on: 67258 67259 67254 67255 67256
Blocks: 64135
  Show dependency treegraph
 
Reported: 2011-08-30 14:59 PDT by Jarred Nicholls
Modified: 2011-09-22 09:54 PDT (History)
7 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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.