Bug 94638 - Merge TestExpectationSerializer into TestExpectationLine
Summary: Merge TestExpectationSerializer into TestExpectationLine
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks: 89161
  Show dependency tree / graph
 
Reported: 2012-08-21 15:16 PDT by Ryosuke Niwa
Modified: 2012-08-22 12:32 PDT (History)
5 users (show)

See Also:

Attachments
Patch (35.97 KB, patch)
2012-08-21 15:21 PDT, Ryosuke Niwa 		dglazkov: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2012-08-21 15:16:37 PDT 
Having a separate class to parse and serialize test expectation doesn't seem to buy us much.
Just let TestExpectationLine serialize itself.
Comment 1 Ryosuke Niwa 2012-08-21 15:21:54 PDT 
Created attachment 159778 [details]
Patch
Comment 2 Dimitri Glazkov (Google) 2012-08-21 17:16:59 PDT 
I thought it was a fairly clean separation of responsibilities. TestExpectationLine is already pretty big. Why do you feel like combining is a good idea?
Comment 3 Ryosuke Niwa 2012-08-21 18:17:17 PDT 
(In reply to comment #2)
> I thought it was a fairly clean separation of responsibilities. TestExpectationLine is already pretty big. Why do you feel like combining is a good idea?

TestExpecations has only 3 functions, two of which are one-liner.

The problem is that TestExpectationLine exposes too much of its internal. It basically doesn't have any encapsulation, and that makes the refactoring to add the support for new format harder.
Comment 4 Dimitri Glazkov (Google) 2012-08-22 11:22:26 PDT 
Comment on attachment 159778 [details]
Patch

ok.
Comment 5 Ryosuke Niwa 2012-08-22 12:32:08 PDT 
Committed r126334: <http://trac.webkit.org/changeset/126334>