The latest SVN bugs thread

[CuteAlien]

Oh my ... there turned up a whole army of off-by-one bugs in the string-class. Anyway, checkout newest svn and your collada should load again.

[hendu]

Surely there are automated tests for things like that, run after every commit

[hybrid]

No, mesh loading tests are mostly not performed, yet. Also because we have not enough freely available meshes, and do not want to distribute lots of unnecessary stuff via SVN. And for the string classes, only those test which are yet written will be run. So we still have a whole bunch of stuff in Irrlicht which is not yet even near some kind of test case.

[hendu]

I mean unit tests for strings alone, no meshes involved, yes.

[hybrid]

Yeah, these exist. But there haven't been any changes in those classes for a long time. I guess, most of these bugs were introduced right at the beginning. And for those methods we have almost no tests. If everything works out perfect, we get a test once we change something in those auxiliary classes. But reality is usually not so nice with us

[CuteAlien]

Mhm, I could have written more tests now as well I guess. I added a bunch of new ones, but oh well.. there's never enough and admittably I even did do some changes now without writing tests for each of them first.

Maybe some day users want tests so much that they start writing tests for all functions ... just daydreaming ;-)

[hendu]

Eh, users expect devs to know their stuff and simply assume there to be 99% test coverage. I know I did

[CuteAlien]

Tests got added rather late to Irrlicht - I think around ~1.4. So most Irrlicht code was written before that. And well, you know how some users want us to write new features all the time instead of spending all our time on tests :-) (although we did spend a _lot_ of time this year already on getting tests working - several months all my Irrlicht spare-time did go into that).

[hendu]

True, true. It's always conflicting demands.

However, I can't deny that I wasn't shocked that there were such fundamental bugs in something as core as string handling, and doubly so that there were no tests for those.

[hybrid]

We don't use much of that stuff in the engine. So often such bugs cannot show up as probably no one uses those functions. That's why we quite often have to fix fundamental bugs in those low-level functions. But the true shock is only due if you find a fundamental bug despite existing test cases

[hendu]

Candidates for removal then?

[CuteAlien]

No, removing functions is bad as you never know what people use. Lets rather concentrate on fixing next bugs. Unless someone has an idea how to get people to volunteer for writing (good!) unit-tests there's nothing we can do to prevent such problems anyway.

[hybrid]

Yeah, moreover it shows that we might be better off using those methods (as soon as they are working correctly ^^ ), it's just that also we have to know about them.

[randomMesh]

Trunk revision 4379 doesn't compile on Linux.

Code: Select all

 
g++ -Wall -pipe -fno-exceptions -fno-rtti -fstrict-aliasing -fexpensive-optimizations -O3 -I../../include -Izlib -Ijpeglib -Ilibpng -I/usr/X11R6/include -DIRRLICHT_EXPORTS=1  -c -o CIrrDeviceStub.o CIrrDeviceStub.cpp
CIrrDeviceStub.cpp: In constructor ‘irr::CIrrDeviceStub::CIrrDeviceStub(const irr::SIrrlichtCreationParameters&)’:
CIrrDeviceStub.cpp:25: error: expected ‘{’ before ‘FileSystem’
CIrrDeviceStub.cpp: At global scope:
CIrrDeviceStub.cpp:25: error: expected constructor, destructor, or type conversion before ‘(’ token
CIrrDeviceStub.cpp:429: error: expected ‘}’ at end of input
make: *** [CIrrDeviceStub.o] Fehler 1
 

[ACE247]

Constructor in CIrrDeviceStub.cpp is meant to look like this:

Code: Select all

 
//! constructor
CIrrDeviceStub::CIrrDeviceStub(const SIrrlichtCreationParameters& params)
: IrrlichtDevice(), VideoDriver(0), GUIEnvironment(0), SceneManager(0),
    Timer(0), CursorControl(0), UserReceiver(params.EventReceiver), Logger(0), Operator(0),
    Randomizer(0), FileSystem(0), InputReceivingSceneManager(0), VideoModeList(0), 
    CreationParams(params), Close(false)