Bug 94638

Summary:

Merge TestExpectationSerializer into TestExpectationLine

Product:

WebKit

Reporter:

Ryosuke Niwa <rniwa>

Component:

Tools / Tests

Assignee:

Ryosuke Niwa <rniwa>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, dglazkov, dpranke, ojan, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

89161

Attachments:

Description	Flags
Patch	dglazkov: review+

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>