Bug 77873

Summary: check-webkit-style failing with "Path does not exist."
Product: WebKit Reporter: Philip Rogers <pdr>
Component: Tools / TestsAssignee: Dirk Pranke <dpranke>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dpranke, levin, ojan, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
test bad paths better none

Description Philip Rogers 2012-02-06 07:31:00 PST
On a fresh checkout check-webkit-style runs without error, but make a space change to LayoutTests/platform/win/test_expectations.txt (which will force check-webkit-style to actually check that file) and the following error will result:

WARNING: Exception while getting port for path LayoutTests/platform/win/test_expectations.txt
WARNING: Could not determine the port for LayoutTests/platform/win/test_expectations.txt. Using 'test' port, but platform-specific expectations will fail the check.
LayoutTests/platform/win/test_expectations.txt:5:  Path does not exist. fast/ruby/after-block-doesnt-crash.html  [test/expectations] [5]
LayoutTests/platform/win/test_expectations.txt:6:  Path does not exist. fast/ruby/after-table-doesnt-crash.html  [test/expectations] [5]
LayoutTests/platform/win/test_expectations.txt:7:  Path does not exist. fast/ruby/generated-after-counter-doesnt-crash.html  [test/expectations] [5]
LayoutTests/platform/win/test_expectations.txt:8:  Path does not exist. fast/ruby/generated-before-and-after-counter-doesnt-crash.html  [test/expectations] [5]
LayoutTests/platform/win/test_expectations.txt:11:  Path does not exist. fast/forms/listbox-clip.html  [test/expectations] [5]
  [ ... snip about 100 lines for brevity ... ]
LayoutTests/platform/win/test_expectations.txt:133:  Path does not exist. fast/table/027.html  [test/expectations] [5]
LayoutTests/platform/win/test_expectations.txt:134:  Path does not exist. fast/table/027-vertical.html  [test/expectations] [5]
Total errors found: 113 in 1 files
Comment 1 Adam Barth 2012-02-06 10:58:25 PST
fast/table/027.html at least does seem to exist.
Comment 2 Ojan Vafai 2012-02-06 11:50:32 PST
At a quick glance, it looks like the problem is that it's not correctly mapping the "win" in the test_expectations.txt path to a port. The "test" port is the fallback port when the port can't be identified. IIRC, Dirk already has a patch somewhere fixing this, but I don't know what the status of it is.
Comment 4 Dirk Pranke 2012-02-06 12:23:00 PST
(In reply to comment #2)
> At a quick glance, it looks like the problem is that it's not correctly mapping the "win" in the test_expectations.txt path to a port. The "test" port is the fallback port when the port can't be identified. IIRC, Dirk already has a patch somewhere fixing this, but I don't know what the status of it is.

Hm. I think Ojan might be referring to bug 76745, which is a bit different (at least, that's the only  patch I've worked on that is ringing a bell for me).

That said, fixing this is straightforward. The mapping from LayoutTests/platform/$name to port is loose at best; the code in lines 74-81 is wrong and what we should do is fetch all of the ports using PortFactory.all_port_names() and then grep the list until you find one whose path_to_test_expectations_file() matches.

I will upload a patch for this shortly.
Comment 5 Dirk Pranke 2012-02-06 16:19:01 PST
Created attachment 125724 [details]
Patch
Comment 6 Dirk Pranke 2012-02-06 16:22:35 PST
Here's a patch that fixes the lookup for all of the real test_expectations.txt files; however, I'm not sure what the right thing to do is if we attempt to check the style for (say) LayoutTests/platform/foo/test_expectations.txt (since there is no foo port) ... the existing code will use the 'test' port, which seems a bit weird at best? It seems like we should either raise an assertion that we're trying to check an unknown file, or have some other sort of no-op checker?
Comment 7 Ojan Vafai 2012-02-06 18:53:01 PST
Comment on attachment 125724 [details]
Patch

Seems fine.
Comment 8 Dirk Pranke 2012-02-08 13:02:12 PST
Created attachment 126137 [details]
test bad paths better
Comment 9 Dirk Pranke 2012-02-08 13:03:07 PST
Committed r107124: <http://trac.webkit.org/changeset/107124>
Comment 10 Philip Rogers 2012-02-08 13:04:48 PST
I just wanted to thank you (Dirk) and Ojan for the quick fixes for these bugs! You guys rock