Unit Testing: Interfaces VS Implementation

[Andreas Pakulat]

On 12.06.11 00:15:38, Milian Wolff wrote:
> Andreas Pakulat, 11.06.2011:
> > On 11.06.11 15:38:54, Milian Wolff wrote:
> > That said, I don't think you need to care about implementations outside
> > of the stable released code, in particular not right now while you get
> > the whole thing up and running.
> 
> I disagree. testing the interface is not possible without a proper 
> implementation imo.

Well, of course you need an implementation - the interface has no code
so there's nothing to test. 

> hence we need to test all implementations, similar to the 
> ModelTest stuff from Qt which one uses to make sure his QAbstractItemModel 
> behaves as it should.

ModelTest does not test 'all implementations'. It simply tests the
model-implementation that you apply it to. Each model will have its own
unit-test that uses the Qt ModelTest class. In the same way cvs, svn,
git etc. all should have their own unit-tests that test exactly that
implementation. Thats the ideal case, unfortunately we live in a
not-so-ideal world so we have to make compromises. One such compromise
is to write an integration test that checks all implementations for some
basic working functionality. 

The downside of such an intergation test is that it covers a lot of
stuff at once, which makes it harder to use for finding bugs and for
adding tests specifically for a bug (so its not re-introduced after
being fixed).

> > > * ProjectModelTest (requires custom [senseless?] code in project model
> > > for the case of items without project/projectmanager, i.e. imo not
> > > actually testing the "real" thing)
> > 
> > No its not senseless, quite the contrary. A test for the project model
> > should only ever test exactly that. It shouldn't test the project class
> > itself (or all the rest of the kdevplatform api). So making the project
> > model easy to plug out of the complete infrastructure is good to write a
> > unit-test for it.
> 
> Then tell me: how do you want to test renaming, moving, ... and all these 
> features which are inherently implementation dependent.

By testing each implementation. The project-model test is there to
verify that the model implementation behaves as expected. If someone
adds move or rename support to the model, then a test for that needs to
be added. But if the rename/move support is added on top of the model
then its tested elsewhere.

> We can easily define some baseline that all implementations should
> follow - but then this should be tested.
> 
> Right now we have some arbitrary custom code in place which gets tested for no 
> real gain as it is never used in production code.

Huh? The dummyproject code in the project model test is not tested at
all, its not part of the unit that is being tested. That one is merely a
mock object so that the test can rely on the project implementation
being bug-free. Thats achieved by having an extremely simple
implementation of the iproject interface.

> > Using the 'real' project impl. and the real projectcontroller impl would
> > mean writing an integration test - to verify that both work properly
> > together.
> 
> Exactly, and that would actually be useful.

Integration tests are useful to verify that certain things are working
together nicely. But that can also mean that this is just by accident
because there's the same bug in both components.

> > One alternative would be to provide a mock implentation of project (and if
> > necessary projectcontroller), that does not have any logic itself and
> > simply provides static values for all its getters.
> 
> Mock implementations are imo just superflous and obsolete.

Sorry to be blunt, but you seem to have some misunderstanding about
the uses for different types of tests and mock objects.

> > As I said above, testing 'everything' is completely out of the scope
> > here. And I think vcsBlackboxTest is the only one testing all
> > implementations (that are installed) of the vcs interfaces. The project
> > model test at least is specifically for the public API and only that.
> > Its not supposed to test the qmake, cmake or custom makefile item
> > subclasses.
> 
> To what avail? True, the projectmodel has some implementation parts which 
> should/could be tested separately, it all boils down to the underlying project 
> manager, esp. the more advanced features (see above).

Sure, you also need tests for the project managers and their code, that
does not mean however that the unit-test for the 1000 lines of project
model code are useless. Quite the contrary as its specialized for the
project model, it makes it very easy to find bugs in that code without
having to hunt through the project manager in addition.

> > > So what I propose is creating some kind of public TestXYZImplementation
> > > class. Implementations can then create a unit test
> > > instantiating/subclassing/implementing this class and run the contained
> > > tests.
> > > 
> > > The advantages:
> > > - no henn-egg problem, we test when we have something that needs to be
> > > tested - it's clear which implementation fails
> > > - the TestXYZImplementation class in KDevplatform is still only checking
> > > the basic interface without any implementation details
> > > 
> > > The cons:
> > > - test class needs to be/have public api (probably?)
> > > - minimal code duplication
> > 
> > I'm not sure I understand what you're trying to achieve with that class.
> > Can you provide some pseudocode?
> 
> ModelTest as we all know it from Qt is just what I'm thinking about.

So this is about testing the interfaces that are provided I assume?
Meaning you'd have a ProjectFileManagerTest that one can give a
IProjectFileManager implementation and then the test will verify that
calling certain things in the public API returns expected results.
Sounds useful, but as the ModelTest triggering an assert there does not
necessarily tell you where the bug is. This kind of thing is good to get
some base-coverage of the code for the common-cases and to make sure to
not do brown-pager-bag releases. 

In many cases however having unit-tests on a more basic level gives you
more precise information about where the bug sits and give you a much
better way of making sure a fixed bug is not re-introduced by later
changes.

Andreas