[Webkit-unassigned] [Bug 69409] [GTK] [WebKit2] Make adding another unit test easier

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Oct 5 09:02:18 PDT 2011


--- Comment #6 from Carlos Garcia Campos <cgarcia at igalia.com>  2011-10-05 09:02:17 PST ---
(In reply to comment #4)
> I think I prefer my original approach in the end for a few reasons:
> 1. Having a generic fixture means that we can apply logic before and after all tests in all suites. The logic for the test path isn't duplicated in multiple places.

There's a generic fixture in my patch too with methods begin and end to do whatever before and after tests.

> 2. I think it's important that we push the most common fixtures now to other files so that they can be used in more than one place. If necessary simple fixtures can be kept inline near tests.
> For an example, I could have easily extended the LoadTrackingTest fixture to also load alternate HTML when a failure happens. Then I could have used that fixture for multiple tests. This test probably belongs in TestWebKitWebView anyway.

That's possible with my patch too, you can just add a new fixture for loading tests and make loader client fixture inherit from it. I merged them just to avoid having an unnecessary library and to make the patch simpler and smaller.

> In the second patch we have to duplicate the WebView and main loop creation. Seeing this in every single WebKit1 unit test file is one of the main reasons I set out to do this. We get to use the C++ class system to our advantage.

You are assuming all tests using a web view have a main loop too. 

> 3. I think we should avoid having the main method as a macro. I dislike macros in general, and perhaps I'll look for a way to implement this via templates.

I guess it's a matter of taste, I like macros :-) 

> 4. I renamed the test files to better match the other files in the source tree. I believe we should use WebKit naming conventions everywhere possible. The only other binaries we produce that are named like this are the WK1 tests.  I'm not sure it makes sense in this case. I'll upload a new patch which fixes build-webkit as well.

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

More information about the webkit-unassigned mailing list