[Webkit-unassigned] [Bug 52136] Add TestOutput classes to webkitpy.

[bugzilla-daemon at webkit.org]

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





--- Comment #6 from Eric Seidel <eric at webkit.org>  2011-01-19 20:59:47 PST ---
(From update of attachment 79548)
View in context: https://bugs.webkit.org/attachment.cgi?id=79548&action=review

> Tools/Scripts/webkitpy/common/net/testoutput.py:95
> +    def __eq__(self, other):
> +        return other != None and self.name() == other.name() and self.type() == other.type() and self.platform() == other.platform() and self.is_actual() == other.is_actual() and self.same_content(other)

Although I don't believe in wrapping just to fit on punch cards, I think some wrapping might make this mor readable.

> Tools/Scripts/webkitpy/common/net/testoutputset.py:70
> +        files = []
> +        for filename in self._zip_file.namelist():
> +            files.append(self._zip_file.open(filename))
> +        return files

This is better written as a list comprehension.

> Tools/Scripts/webkitpy/common/net/testoutputset.py:98
> +        outputs = []
> +        for image_file in image_files:
> +            checksum_file_name = re.sub(re.compile('\.png'), '.checksum', image_file.name())

Anytime you're using this pattern, a helper function + a list comprehension is normally cleaner.

> Tools/Scripts/webkitpy/common/net/testoutputset.py:128
> +        outputs = []
> +        for builder in self._builders:
> +            outputs += builder.outputs_for(name, **kwargs)
> +        return outputs

list comprehension.

> Tools/Scripts/webkitpy/common/net/testoutputset_unittest.py:35
> +    def __str__(self):
> +        return "FakeZip"

Why override __str__?

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