Bug 53356 - test-webkitpy: fix webkitpy.layout_tests.port.mac_unittest.MacTest.test_skipped_file_paths
Summary: test-webkitpy: fix webkitpy.layout_tests.port.mac_unittest.MacTest.test_skipp...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Dirk Pranke
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-28 16:56 PST by Dirk Pranke
Modified: 2011-01-28 18:08 PST (History)
5 users (show)

See Also:


Attachments
Patch (2.40 KB, patch)
2011-01-28 16:57 PST, Dirk Pranke
no flags Details | Formatted Diff | Diff
make version-independent per review feedback from mihaip (3.15 KB, patch)
2011-01-28 17:54 PST, Dirk Pranke
mihaip: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Pranke 2011-01-28 16:56:31 PST
test-webkitpy: fix webkitpy.layout_tests.port.mac_unittest.MacTest.test_skipped_file_paths
Comment 1 Dirk Pranke 2011-01-28 16:57:17 PST
Created attachment 80526 [details]
Patch
Comment 2 Dirk Pranke 2011-01-28 17:01:27 PST
This is one way to fix the test that adam disabled in r76428. It takes the approach that this is a platform-dependent "integration test" and will work correctly for the version of the Mac it is running on.

An alternative would be to rewrite this test to be version-independent and simply compare against what we think the list of skipped files is supposed to be for whatever versions we wish to support. 

I don't have a strong leaning either way. Thoughts?
Comment 3 Mihai Parparita 2011-01-28 17:31:16 PST
Comment on attachment 80526 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=80526&action=review

> Tools/Scripts/webkitpy/layout_tests/port/mac_unittest.py:46
>      # FIXME: This test does not appear to be correct. It seems to receive

Should remove this comment.

> Tools/Scripts/webkitpy/layout_tests/port/mac_unittest.py:60
> +            expected_paths.append('/LayoutTests/platform/mac' + port.version() + '/Skipped')

Assuming that we had complex fallback logic (e.g. mac-snowleopard falls back on mac-leopard, but mac-leopard doesn't fall back mac-tiger), then it seems like we'd want to force port.version() to fixed value(s), so that we could get full coverage of those combinations regardless of what platform the test is run on.

However, we don't actually have complex fallback logic. Still, I think overriding version() is the way to go.
Comment 4 Dirk Pranke 2011-01-28 17:54:05 PST
Created attachment 80532 [details]
make version-independent per review feedback from mihaip
Comment 5 Dirk Pranke 2011-01-28 17:54:50 PST
(In reply to comment #3)
> Still, I think overriding version() is the way to go.

Ok.
Comment 6 Mihai Parparita 2011-01-28 17:56:10 PST
Comment on attachment 80532 [details]
make version-independent per review feedback from mihaip

View in context: https://bugs.webkit.org/attachment.cgi?id=80532&action=review

> Tools/ChangeLog:8
> +         handles the version of the platform it is running on correctly.

ChangeLog should be updated since the platform that it's running on no longer matters.
Comment 7 Dirk Pranke 2011-01-28 18:08:24 PST
Committed r77039: <http://trac.webkit.org/changeset/77039>