[Webkit-unassigned] [Bug 68572] Create unit tests for LayerChromium

[bugzilla-daemon at webkit.org]

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





--- Comment #13 from James Robinson <jamesr at chromium.org>  2011-09-27 12:56:01 PST ---
(In reply to comment #7)
> >> Source/WebKit/chromium/tests/LayerChromiumTest.cpp:83
> >> +        EXPECT_CALL(silentDelegate, notifySyncRequired()).Times(AnyNumber());
> > 
> > isn't this exactly the same as passing 0 in for the delegate?
> 
> in my opinion -  A "mock delegate" and "no delegate at all" are not the same - LayerChromium should believe it has a working delegate, but we don't care how it uses the delegate.   In our particular implementation of LayerChromium, yes they would produce the same "test results", but that's only a side effect of how we programmed the LayerChromium.  For example, if later we find its useful to add an early-exit if m_delegate is NULL...  Then we may not be testing what we think we are testing.  Using a stub seems less brittle to me.  Thoughts?

I don't think that's true.  We create LayerChromiums will NULL delegates and NULL out the delegate on LayerChromiums and expect that to behave the same way as having a no-op delegate.

> 
> >> Source/WebKit/chromium/tests/LayerChromiumTest.cpp:86
> >> +    void explicitlyDestroyLayer(PassRefPtr<LayerChromium> layer) const
> > 
> > This appears to be redundant with LayerChromiumWithInstrumentedDestructor.  Is there any reason to have both?
> 
> I think its important that we directly test LayerChromium when possible.  Testing the subclass could be misleading if we introduce errors into the subclass.  However, to make the leak test, we had no choice but to passively check the destructor is called, and that should be the only test that uses this subclass.
> 

I think having a subclass that only provides a destructor is perfectly valid to use in tests where you want the subclass to behave the same in every way except for the destruction behavior.

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