[prev in list] [next in list] [prev in thread] [next in thread] 

List:       koffice-devel
Subject:    Re: ODF text (was Release schedule after the alpha's)
From:       Thomas Zander <zander () kde ! org>
Date:       2007-12-20 9:21:50
Message-ID: 200712201021.53544.zander () kde ! org
[Download RAW message or body]

[Attachment #2 (multipart/signed)]


On Thursday 20 December 2007 02:00:14 Sebastian Sauer wrote:
> Thomas Zander wrote:
> > I think having unit tests for loading is an absolute must, but every
> > time I try I'm lost in the amount of classes that have to be
> > instantiated to be able to actually call something like
> > KoTextLoader::loadList
>
> y, I also agree there. Well, an excuse may that we did follow the
> XP-development but did left out the chapter that deals with unittests
> ;)
>
> > I'd like to suggest two things
> >
> > 1) refactor the context objects to have no arguments to their
> > constructors unless they are required for the context objects
> > constructor to run. Add setFoo() methods on those classes.
>
> sounds for me more like a cosmetic change which could be for sure done
> rather fast but may not provide a the best solution :-/

From reading your whole mail, I think its not cosmetic, its due to 
different approaches to testing. And how what I've learned to do testing 
its very important to have a very cheap way to instantiate a class with 
the option to set extra objects in there, as is required for the test.
I'll explain more in depth what I mean later on, but I just wanted to 
point out that this change in code makes the code more easily unit 
tested. (read on for why)

> So, you may also note that most of 
> the code is within one huge class. 

Yeah, I agree that it makes things easier to find. I love the subdir idea 
as well ;)
> That#s for 2 reasons; 
[]
> 3. parts of the code...
*grin* don't you hate it when you come up with a 3th method half way 
through writing ! :P

> > And then almost all methods on KoTextLoader you get have to pass in
> > an instance of KoTextLoadingContext. The one you created *with* the
> > loader as a reference.
>
> Nah, and that's where I did look at the code :) and...
> seems it's not used at all. 

Low hanging fruit, always good. And good to know my brief inspection of 
the code has already yielded one optimization.

> > Bottom line; I want to have a way to test this.  In my test i need a
> > way to instantiate the correct classes.
> > Define an XML that I then let KoXml load, as my testing code.
> > Then I want to call specific methods in the KoTextLoader and check if
> > the QTextDocument looks correct afterwards.
> >
> > Who can help me with that? I already added an empty test framework to
> > be filled ;)

On Thursday 20 December 2007 02:13:36 Sebastian Sauer wrote:
> > please just commit it, so I can discover probably other points that
> > are not logical / missing :)
>
> Thinking again about it; such a unittest-suite would probably also link
> against KWord since kotext does not contain all the code needed to
> load+display a odt-document (it misses all those parts that are within
> kword/part/KWOpenDocumentLoader.* ). So, to "check if the QTextDocument
> looks correct afterwards" I would suggest to just use kword ;) imho a
> "real"(TM) unittest would need to do it like the KHTML-testsuite does.

This is now what I had in mind when I said I wanted to unit test.
In my years of writing unit tests I noted that its very useful to make the 
unit of testing as small as possible. Typically one method in a class. 
The idea is to make sure you execute a minimum amount of code outside of 
that class while doing your testing to make sure you are testing *only* 
the coding-unit, and not more.
To do what you write above requires instantiating a lot more classes, and 
running through more code that will hopefully call the method you want to 
test. Which means that if in the basic loading code something fails; a 
lot of tests fail. Which is a bit counter productive as you then still 
have to guess where in the code something broke.

Well, I guess if you go to websites that discuss XP they can explain it 
much better than I can.

So, my idea of unit testing the ODF-text loading code is to setup a 
minimum of data in each test. Typically just an xml stream and the 
context as well as a QTextDocument, and then call the loadList() method 
directly. For example.
Afterwards you can inspect the data classes to see if the right thing 
happened.

So, for that reason I want to
a) find the code sequence fully typed out in one standalone file that 
allows me to call the loadFoo()
b) refactor the code (like the constructor stuff at the top of this mail) 
to allow me to instantiate less classes against which I'll test.

So, if you can help out with (a) at least, then I'd love to do that 
couple-of-hours-of-refactoring soon as well :)

Lets make this code that's fun to hack on!
-- 
Thomas Zander

[Attachment #5 (application/pgp-signature)]

_______________________________________________
koffice-devel mailing list
koffice-devel@kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel


[prev in list] [next in list] [prev in thread] [next in thread]