Bug 67006

Summary: [chromium] Add MockWebGraphicsContext3D, for compositor unit testing
Product: WebKit Reporter: Iain Merrick <husky>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, enne, husky, kbr, nduca, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Iain Merrick
Reported 2011-08-25 17:54:11 PDT
[chromium] Add MockWebGraphicsContext3D, for compositor unit testing
Attachments
Patch (29.54 KB, patch)
2011-08-25 18:01 PDT, Iain Merrick
no flags
Iain Merrick
Comment 1 2011-08-25 18:01:39 PDT
Nat Duca
Comment 2 2011-08-25 18:36:36 PDT
Comment on attachment 105288 [details] Patch I have a high level question for @kbr, though, which is whether this is the right layer to hook at. I just can't make my mind up, personally. Clearly, its the only place we can hook right now because everything higher is non-virtual. However, this means that all our testing becomes specific to the Chromium port. Is this OK? I guess some of this ties back to me not understanding WebKits' overal position on unit tests...
Kenneth Russell
Comment 3 2011-08-25 19:09:32 PDT
(In reply to comment #2) > (From update of attachment 105288 [details]) > I have a high level question for @kbr, though, which is whether this is the right layer to hook at. I just can't make my mind up, personally. > > Clearly, its the only place we can hook right now because everything higher is non-virtual. > > However, this means that all our testing becomes specific to the Chromium port. > > Is this OK? I guess some of this ties back to me not understanding WebKits' overal position on unit tests... I think this is fine for the time being. There's been discussion in recent months about settling on a unit testing framework for the overall WebKit project -- see https://lists.webkit.org/pipermail/webkit-dev/2011-April/016510.html -- but so far everyone's been too busy to push it forward. There was some consensus at the last WebKit meeting in April that the project would try to standardize on gtest, which is full featured and well maintained, so adding a gtest-compatible mock and tests sounds good even if they're currently Chromium specific. They could be made port-independent in the future.
Kenneth Russell
Comment 4 2011-08-25 19:16:00 PDT
Comment on attachment 105288 [details] Patch This generally looks nice. Hopefully this will allow us to eventually catch some errors earlier. I am concerned though that adding new methods to WebGraphicsContext3D (which happens reasonably frequently) will cause the WebKit unit tests to break and developers won't catch that until fairly late. Do the Chromium EWS bots build and run the webkit unit tests?
Nat Duca
Comment 5 2011-08-25 21:15:51 PDT
Hmm, the builders do, but I'm not sure how they relate to EWS... +abarth :)
Nat Duca
Comment 6 2011-08-26 11:10:05 PDT
@kbr, can you do a formal review of this? I'm going to pick up Iain's patch and try to land it before I land 66807.
Kenneth Russell
Comment 7 2011-08-26 16:19:06 PDT
Comment on attachment 105288 [details] Patch OK. If we find that we're breaking things frequently on the canaries we can change what gets compiled in which targets. r=me
WebKit Review Bot
Comment 8 2011-08-26 17:41:46 PDT
Comment on attachment 105288 [details] Patch Clearing flags on attachment: 105288 Committed r93928: <http://trac.webkit.org/changeset/93928>
WebKit Review Bot
Comment 9 2011-08-26 17:41:51 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.