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

[bugzilla-daemon at webkit.org]

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





--- Comment #12 from James Kozianski <koz at chromium.org>  2011-01-23 21:34:40 PST ---
(In reply to comment #11)
> (From update of attachment 79684 [details])
> 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.)?

LayoutTests seem to be the most prominent kind of tests (in webkitpy if not WebKit), so I think it would be okay to leave it unadorned, and to prefix the other TestOutput classes if we need to introduce them.

> 
> 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?

Right - test the noun and test the verb. I think Check is a good substitute for test as a verb.

> 
> 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.

Yep, I've clarified the comment.

> 
> > 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?

This class is actually quite generic. It can represent a set of test outputs from a buildbot, or it could be the outputs for a particular test's run, or it could be the -expected outputs in a LayoutTest directory.

> 
> > 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.

Done.

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

Done.

> 
> 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.

Done, and I renamed FakeZip to MockZip to fit in with MockFileSystem.

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