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

[bugzilla-daemon at webkit.org]

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





--- Comment #11 from Dirk Pranke <dpranke at chromium.org>  2011-01-21 15:43:51 PST ---
(From update of attachment 79684)
View in context: https://bugs.webkit.org/attachment.cgi?id=79684&action=review

> Tools/Scripts/webkitpy/common/net/testoutput.py:34
> +class TestOutput(object):

Do we need to call this LayoutTestOutput to distinguish it from other kinds of tests (e.g., JSC tests, Perf tests, etc.)?

Also, I think I've mentioned this before, in another bug/context, but we use the word "test" to refer to two different kinds of things. 

The first is a given Layout Test (e.g., fast/html/article-element.html) -- that's the sense being used here -- and the second is one particular kind of check against the output. I.e., checking the text output, checking the image output, etc. The latter shows up in layout_tests/layout_package/test_failures, layout_tests/test_types, and in a few other places. I'd really like to find a different noun for the latter kind of thing, but have yet to think of anything good. "Check" maybe?

At any rate, I'm looking for suggestions. That said, we might want to update the docstring to this class to be clear we're talking about all of the related "checks" for a single "test". It's possible to read this description and get slightly confused otherwise.

> Tools/Scripts/webkitpy/common/net/testoutputset.py:33
> +    """A set of test outputs"""

Maybe add a comment here that the set of test outputs is usually (always?) from a whole test run?

> Tools/Scripts/webkitpy/common/net/testoutputset_unittest.py:50
> +        print "FakeZip> cp %s %s" % (filename, path)

Do you need these print statements (here and in delete())? I don't see you capturing stdout output anywhere, but maybe I'm missing something. If not, you should probably delete them because generally we don't like our unit tests to print anything.

Also, maybe consider passing in a filesystem_mock object to the constructor and having extract write to that?

I need some zip-wrapper objects for writing tests for the rebaseline-chromium-webkit-tests script so we should use the same objects if possible. In which case, I'd want FakeZip to be in a separate module so I can import it.

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