#256 [PATCH][v1] Ellipsoid API unit test

[Andrei Popescu]

I've added an API test just for the Ellipsoid class.

Some questions:
- is the general test structure acceptable?
- should the methods be tested for correctness or just functionality
- there is a TODO left in code, I'm open for suggestions

I'll probably get a better idea and finish this test after some feedback on this patch

Thanks,
Andrei

[Daniel Roßberg]

I'm missing the database here. How about a structure like this:

AllPrimitives.cpp >>>>>
MakeArb8(){}
MakeEllipsoid(){}
...
main(){
BRLCAD::FileDatabase database(argv[1]);
MakeArb8(database);
MakeEllipsoid(database);
...
}
<<<<<<<<<<

Tests for functionality should be enough here, but you could check for IsValid() for every primitive.
The result will be a BRL-CAD geometry file which contains examples of every primitive type supported by this interface.

Daniel

[Andrei Popescu]

Daniel, I have changed the unit test according to what I understood from your comments.

Andrei

[Daniel Roßberg]

Does this compile?

[Andrei Popescu]

oops, forgot to remove the Save, but aside of it it does.

[Daniel Roßberg]

OK, next step: Copyright 2011?

To make point out what the program does I would like to see the different tests in own functions (e.g. TestArb8()). Then the main() consists of calls of these functions. This makes clear what will be tested there and there would be no need to mention the teted primitives in the comment then. When you implemented a new primitive it's easy to add a test for it.

Daniel

[Andrei Popescu]

Daniel,

I fixed the copyright issue as well as broken down the test into small functions, one per primitive.

Andrei

[Daniel Roßberg]

Before we fine-tune it you should fix the copy-n-paste remnant "std::cout << database.Title();". It doesn't make sense here.

In general the patch would be OK then. However, we still have to settle to the coding conventions.

Daniel

[Andrei Popescu]

Fixed

[Andrei Popescu]

Daniel,

I've added simple tests for new primitives(except pipe, since it s not yet accepted, however, I ran into a bit of trouble, I couldn't use rtgeom.h, for some reason. (I needed Vector3D)

Thanks

[Daniel Roßberg]

with revision 63634