Menu
Logged-In As
ACCOUNTNot Logged In
Create unit tests for plane.c point functionsBRL-CAD
Status: ClosedTime to complete:
72 hrs
Mentors: Sean
There are more than 300 library functions in our core LIBBN library. As a core library used by nearly every one of BRL-CAD's tools, testing those functions for correct behavior is important.
References:
- include/bn.h
- include/plot3.h
- include/vmath.h
- src/libbn/qmath.c
- src/libbu/tests/*.c (note libbu, not libbn for examples)
- src/libbn/tests/*.c
Code:
- src/libbn/tests/qmath.c
- src/libbn/tests/CMakeLists.txt
This task involves implementing a new unit test for any of LIBBN's plane.c public point functions:
grep -E '^bn' plane.c | sort | cut -f1 -d\( | sort | grep pt | grep -v _dist_ | grep -v _isect_
Uploaded Work
| File name/URL | File size | Date submitted | |
|---|---|---|---|
| plane_pt.diff | 12.7 KB | November 26 2013 18:57 UTC | |
| plane_pt_rev2.diff | 12.6 KB | November 26 2013 23:37 UTC | |
| plane_pt_rev3.diff | 12.6 KB | November 26 2013 23:53 UTC | |
| plane_pt_rev4.diff | 12.4 KB | November 28 2013 15:38 UTC | |
| plane_pt_rev5.diff | 12.4 KB | November 29 2013 15:21 UTC |
I would like to work on this task.
This task has been assigned to Andromeda Galaxy. You have 72 hours to complete this task, good luck!
Melange has detected that the initial deadline has passed and it has set the task status to ActionNeeded. The student has 24 hours to submit the work before the task is reopened and sent back to the pool for other students to claim.
Melange has detected that the final deadline has passed and it has reopened the task.
I would like to work on this task.
This task has been assigned to Andromeda Galaxy. You have 72 hours to complete this task, good luck!
The work on this task is ready to be reviewed.
One of the mentors has sent this task back for more work. Talk to the mentor(s) assigned to this task to satisfy the requirements needed to complete this task, submit your work again and mark the task as complete once you re-submit your work.
First, I'd like to say great work, this is a very good patch, but it's not quite perfect yet.
Typically, a patch is generated from the projects top source directory, otherwise we don't know where to apply it (there was no instruction to cd to src/libbn/tests/ before applying).
The CHECK_RESULT macro damages readability, I think. When a function ends with that macro, my first feeling is that the function is wrong because it doesn't actually return a value, it takes digging to find it. One could reduce the macro into a substitution so it could be returned, and at that point, the macro doesn't feel like it adds anything at all.
The POINT_SCANF_GEN macro seems a bit off to me. Instead of a macro to scan to floats (that are not macro local) and shoving them into doubles, why not just scan into doubles (using %lf)?
I'd argue that neither macro makes the code more readable, so both should be removed...
Otherwise, excellent work!
-Erik
The work on this task is ready to be reviewed.
Andromeda, this is looking fantastic but I did notice a few issues that need to be corrected in order to apply the patch. Still overall, this looks really good. Hope you tackle some more test cases!
The issues I saw on quick review were:
One of the mentors has sent this task back for more work. Talk to the mentor(s) assigned to this task to satisfy the requirements needed to complete this task, submit your work again and mark the task as complete once you re-submit your work.
The work on this task is ready to be reviewed.
One issue remaining that I didn't catch the first time: commas. See our HACKING file for several examples, but basically a space is expected after all of the C-syntactic commas (i.e., not those related to scanf parsing). Here are a couple examples where you compress them:
bn_mk_plane_3pts(plane,a,b,c,tol);
return test_bn_3pts_collinear(argc,argv);
Since you are relying on compressed form for scanf parsing, that means we cannot auto-format this file so you'll need to manually review all of your commas to ensure there's a space after all of them except for those related to input scanning.
The rest fo the changes look spot-on. Nice work. I also realize now that I'd suggested V3ARGS but you cannot (presently) use that with sccanf-style arguments. If you like, I can create a follow-on task to fix that. ;)
One of the mentors has sent this task back for more work. Talk to the mentor(s) assigned to this task to satisfy the requirements needed to complete this task, submit your work again and mark the task as complete once you re-submit your work.
The deadline of the task has been extended with 1 days and 0 hours.
The work on this task is ready to be reviewed.
Congratulations, this task has been completed successfully.
Andromeda, now that looks pretty good. Only found two issues. I ran into a conflict patching src/libbn/tests/CMakeLists.txt so make sure that you do run "svn up" and resolve any conflicts prior to creating your patches with "svn diff". Also, the test binary segfaults/crashes if you run it without any arguments (make src/libbn/tests/tester_bn_plane_pt). I applied fixes to both starting with r58716, you're encouraged to review them for future reference.
What's next? :)