Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve test and testutilities. #150

Merged
merged 23 commits into from
Nov 4, 2014
Merged

Improve test and testutilities. #150

merged 23 commits into from
Nov 4, 2014

Conversation

Makman2
Copy link
Owner

@Makman2 Makman2 commented Aug 25, 2014

Also add make_zero() functions to make near zero values to absolute zero.
Needed for relative compares.

@sils
Copy link
Collaborator

sils commented Aug 30, 2014

ah this covers the commits from the other pull request? Can you close the other pull request if its obsolete then?

@sils
Copy link
Collaborator

sils commented Aug 30, 2014

most commit messages are too long, especially the first line. Try to hold 50 chars max! In some cases it might sense to go for up to 70 but only when theres no way to strip something off the line.

@sils
Copy link
Collaborator

sils commented Aug 31, 2014

other commits seem good on first glance except commit message ;) (trailing dot)

Makman2 added 3 commits August 31, 2014 16:54
Test utilities are a useful tool for every test,
so automatically adding it to every test suite
makes it easier for the user, so he need not to
write everytime the test utilities dependency into
the CMakeFiles.
testutilities.cpp itself points to the wrong
header file.
Migrating from the old transformation_test
functions to the new universal testutilities.
The affected tests are:
- Boost extensions test
- Transformation::Translation test
- Transformation::OrthogonalProjection test
- Transformation::Custom test
- Transformation::Identity test
- Transformation::TransformationChain test
@Makman2
Copy link
Owner Author

Makman2 commented Aug 31, 2014

Because of the rebase of testimprove the whole tree get diverged and I must rebase it completely from ground up (also to apply your fixes).
I make several rebases to not forget anything. So please wait with review until I say it's over :)

@Makman2 Makman2 force-pushed the testimprove2 branch 2 times, most recently from ac276de to d8ee524 Compare August 31, 2014 16:23
@Makman2
Copy link
Owner Author

Makman2 commented Aug 31, 2014

Done.

@Makman2 Makman2 removed the blocked label Aug 31, 2014
@Makman2
Copy link
Owner Author

Makman2 commented Aug 31, 2014

I hope I didn't forget something. I just realized that the discussions are also removed when force-pushing, before I thought to have seen that they will be kept.

@sils
Copy link
Collaborator

sils commented Aug 31, 2014

you forgot some trailing dots in commit shortlog here and there. would you mind correcting that before I continue review?

@Makman2
Copy link
Owner Author

Makman2 commented Aug 31, 2014

So tested the BOOST_CHECK macros: make attempts an error (Error 2).

@sils
Copy link
Collaborator

sils commented Aug 31, 2014

sounds like it should. good then I'm in for this.

Makman2 added 6 commits August 31, 2014 19:37
Replace REQUIRE with CHECK, so tests do not abort
and next features can be checked. Only
construction tests remain REQUIRE because all
other tests would fail if the tested object
can't be constructed.
Saves writing boost::numeric::ublas::...
To check if two matrices/vectors are equal it's
not neccessary to copy the content of it. Just
check them in-place.
@sils
Copy link
Collaborator

sils commented Oct 14, 2014

thats good looking commit messages!

@Makman2
Copy link
Owner Author

Makman2 commented Oct 20, 2014

So everything settled for merge?

@Makman2
Copy link
Owner Author

Makman2 commented Oct 24, 2014

Okay that should be all fixes. If you ack'ed them I'll rebase and merge some commits as you suggested.

Makman2 added 10 commits November 3, 2014 13:42
Move orthogonal depth projection class
construction test to new test suite for this
class. Orthogonal depth projection test
has nothing lost in the test for
orthogonal projection.
This function sets near zero values to absolute
zero. Needed for testing and checking purposes.
The functions change to a generic version so that
the functions are usable with different versions
of the boost test assert macros. The BOOST_REQUIRE
is not needed every time, sometimes BOOST_CHECK
is the better alternative or even BOOST_WARN.
Add make_zero() function that makes near-zero
values to absolute zero. This signature supports
vector types.
Change test name because of multiple make_zero()
function overloads.
Add @tparam for make_zero() functions for matrix
and vector types.
This make_zero() function supports all numeric
expressions and makes them to absolute zero if
near zero.
Every index type can be accessed from the vector
itself (via VectorType::size_type), so
ModelIdxType is not needed anymore.
@Makman2
Copy link
Owner Author

Makman2 commented Nov 3, 2014

So you can see what changed (I will push more often the rebase-script when rebasing):

pick aa159d9 test: Improve world construction test
fixup 8d663d0 test: Remove empty '#'
pick 950454d test: Move test-case to new test suite
pick 4b58310 test: Improve documentation for RandomMatrix()
pick 187b7be boostext: Add new function make_zero()
pick ac735eb buildsystem: Remove unneeded CMake dependency
pick 0c56136 test: Rewrite testutilites test functions
pick df04065 boostext: Add make_zero function for vectors
pick 31c4d62 test: Change test name
pick 2583e98 test: Improve documentation
pick 3ea72d3 boostext: Add make_zero() function for numerics
pick e674cc9 std: Remove typedef for vector index type
pick fc70539 test: Make optional parameter
pick f4e0e3d testutilities: Make RandomMatrix() more random
pick 9234c73 test/transformation_chain: Use new RandomMatrix()
pick 688925a testutilities: Description for relative checking

@Makman2 Makman2 removed the blocked label Nov 3, 2014
@sils
Copy link
Collaborator

sils commented Nov 4, 2014

you can merge this after having squashed those commits I pointed out

@Makman2
Copy link
Owner Author

Makman2 commented Nov 4, 2014

pick 23dbdbb testutilities: Make RandomMatrix() more random
fixup b7725de test: Make optional parameter
pick 4cfd267 test/transformation_chain: Use new RandomMatrix()
pick 60aca06 testutilities: Description for relative checking

@Makman2 Makman2 removed the blocked label Nov 4, 2014
@Makman2
Copy link
Owner Author

Makman2 commented Nov 4, 2014

Ah damn... The optinal parameter should go away...
It won't work either because of the overload...

@Makman2 Makman2 added the blocked label Nov 4, 2014
Makman2 added 3 commits November 4, 2014 20:33
Delete default parameter for function and create
overload that passes seed from time in total
seconds.
Use the new and more random RandomMatrix()
function to have a more random and better test.
Improve the docs for `IsMatrixEqual()` and
`IsVectorEqual()` + explain why used relative
checking in function bodies and not absolute.
@Makman2
Copy link
Owner Author

Makman2 commented Nov 4, 2014

edit 6212c6b testutilities: Make RandomMatrix() more random
pick 0a81702 test/transformation_chain: Use new RandomMatrix()
pick 9c0225a testutilities: Description for relative checking

sils added a commit that referenced this pull request Nov 4, 2014
Improve test and testutilities.
@sils sils merged commit 1ce0e5c into master Nov 4, 2014
@sils sils deleted the testimprove2 branch November 4, 2014 19:35
@Makman2 Makman2 removed the blocked label Nov 4, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants