[Webkit-unassigned] [Bug 42832] webkit-patch command to find the ports covering a specific layout test

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=42832


Adam Barth <abarth at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #62405|review?                     |review-
               Flag|                            |




--- Comment #9 from Adam Barth <abarth at webkit.org>  2010-07-24 08:43:31 PST ---
(From update of attachment 62405)
WebKitTools/Scripts/webkitpy/common/checkout/scm.py:244
 +      def directory_contents(self, path):
Please add unit tests in scm_unittests.  To run the SCM unit tests, you need to pass --all to test-webkitpy.

WebKitTools/Scripts/webkitpy/tool/commands/queries.py:301
 +                           if line.strip() and not line.startswith("#")]
This calls line.strip twice.  :(

WebKitTools/Scripts/webkitpy/tool/commands/queries.py:307
 +                      tests_skipped.append(test_name)
skipped_tests and tests_skipped are very similar names.  It's hard to know which is which.

WebKitTools/Scripts/webkitpy/tool/commands/queries.py:297
 +      def _find_tests_in_skipped(self, scm, tests, skipped_file):
We shouldn't be parsing Skipped files at the command layer.  That's something much lower level.  We need to add some facilities in one of the lower-level modules so that we can re-use this parsing code in other commands.

WebKitTools/Scripts/webkitpy/tool/commands/queries.py:319
 +          chromium_expectations_path = os.path.join(platform_directory, "chromium", "test_expectations.txt")
The command layer shouldn't know anything about specific ports.  All that information needs to be encapsulated in the port abstraction.

WebKitTools/Scripts/webkitpy/tool/commands/queries.py:321
 +                   if line.find("SKIP") >= 0 and line.strip() and not line.startswith("//")]
We already have a very nice facility for parsing test_expectations.  Please don't re-invent the wheel.

WebKitTools/Scripts/webkitpy/tool/commands/queries.py:350
 +          scm = tool.scm()
We don't generally store scm in a local variable.  We just access it off the tool.

WebKitTools/Scripts/webkitpy/tool/commands/queries.py:351
 +          platform_directory = tool.scm().absolute_path(os.path.join("LayoutTests", "platform"))
This knowledge doesn't belong at the command layer.

WebKitTools/Scripts/webkitpy/tool/mocktool.py:438
 +  # I am a Skipped file
This sort of fixture belongs in the unit test for a module that understands Skipped files.  These fixtures are a bit higher level.  That's just a symptom of trying to put too much at the command layer.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.