[webkit-reviews] review granted: [Bug 37393] Sketch out the win port for new-run-webkit-tests : [Attachment 53057] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Apr 10 16:31:20 PDT 2010


Eric Seidel <eric at webkit.org> has granted Adam Barth <abarth at webkit.org>'s
request for review:
Bug 37393: Sketch out the win port for new-run-webkit-tests
https://bugs.webkit.org/show_bug.cgi?id=37393

Attachment 53057: Patch
https://bugs.webkit.org/attachment.cgi?id=53057&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
 43		if options and hasattr(options, 'chromium') and
options.chromium:

should just be options.chromium, no?

What's this used for?
190191	   def test_base_platform_names(self):

are you sure it shoudl return "win" as well as mac?

God I had 80c wrap:
245	    return [os.path.join(self._webkit_baseline_path(self._name,
 246								 'Skipped'))]

What's this used for?  Is this right?
84     def test_platform_name(self):
280	     raise NotImplementedError('WebKitPort.test_platform_name')
 285	     return self._name + self.version()

Why?
48     def version(self):
 49	    return ''

Portname override is lame:
43     def __init__(self, port_name=None, options=None):
 44	    if port_name is None:
 45		port_name = 'win'

 63	def _tests_for_disabled_features(self):
 is a copy/paste disaster.  Can't that be pushed into WebKitPort?

Please respond to the comments.  Otherwise this looks like an OK first start.


More information about the webkit-reviews mailing list