-
Notifications
You must be signed in to change notification settings - Fork 86
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
[WIP] Feature/tests #108
base: master
Are you sure you want to change the base?
[WIP] Feature/tests #108
Conversation
Signed-off-by: Julien 'Lta' BALLET <[email protected]>
Signed-off-by: Julien 'Lta' BALLET <[email protected]>
Signed-off-by: Julien 'Lta' BALLET <[email protected]>
AM_CONDITIONAL([DEBUG], [test x"$enable_debug" == x"yes"]) | ||
AM_CONDITIONAL([PROGRAMS], [test x"$enable_programs" != x"no"]) | ||
AM_CONDITIONAL([TESTS], [test x"$enable_tests" != x"no"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this imply default=yes ?
AC_ARG_WITH([urandom], | ||
[AS_HELP_STRING([--with-urandom=PATH], | ||
AC_ARG_WITH([urandom], | ||
[AS_HELP_STRING([--with-urandom=PATH], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpicking, but unrelated
@@ -113,6 +113,17 @@ dsm_inverse_SOURCES = bin/inverse.c | |||
|
|||
dsm_lookup_SOURCES = bin/lookup.c | |||
|
|||
if TESTS | |||
bin_PROGRAMS += all_tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the test scenario you're aiming for something like
- ../configure --enable-tests
- make
- ./tests
Or is make check considered?
To be honest I don't have a strong opinion on wether make check should be used when it's not actually running the tests, but this means writing a test harness for cmocka IIUC
@@ -0,0 +1,14 @@ | |||
#include <stdarg.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: Missing license header
Seems a great idea! |
As discussed quite some time ago with @jbkempf, I said I would start to add a bunch of tests to this little beast.
Very little of the interesting parts of the code can be tested this way, so it'll need to be paired with a higher level integration testing suite. But this is already something.