Hello,
I recently introduced new utility function sss_utc_to_time_t(). This patches should refactor some existing code to reuse it.
1st patch - add unit tests for fuctions to be amended. 2nd patch - amends check_pwexpire_kerberos() and modifies test - to clearly show how has behavior changed. 3rd patch - amends nds_check_expired() - no test changes required
ci of this version of patches has passed. http://sssd-ci.duckdns.org/logs/job/10/95/summary.html
Thanks!
On Mon, Mar 16, 2015 at 12:10:02PM +0100, Pavel Reichl wrote:
Hello,
I recently introduced new utility function sss_utc_to_time_t(). This patches should refactor some existing code to reuse it.
1st patch - add unit tests for fuctions to be amended. 2nd patch - amends check_pwexpire_kerberos() and modifies test - to clearly show how has behavior changed. 3rd patch - amends nds_check_expired() - no test changes required
ci of this version of patches has passed. http://sssd-ci.duckdns.org/logs/job/10/95/summary.html
Thanks!
The refactoring looks good to me but I don't like the tests :-)
From c323db03ff2c72dbfe6496babe2c0920a412118c Mon Sep 17 00:00:00 2001 From: Pavel Reichl preichl@redhat.com Date: Fri, 6 Mar 2015 04:29:24 -0500 Subject: [PATCH 1/3] TESTS: test expiration
Makefile.am | 44 ++++++++++++ src/providers/ldap/sdap_access.c | 2 +- src/tests/cmocka/test_expire_common.c | 130 ++++++++++++++++++++++++++++++++++ src/tests/cmocka/test_expire_common.h | 40 +++++++++++ src/tests/cmocka/test_ldap_auth.c | 99 ++++++++++++++++++++++++++ src/tests/cmocka/test_sdap_access.c | 71 +++++++++++++++++++ 6 files changed, 385 insertions(+), 1 deletion(-) create mode 100644 src/tests/cmocka/test_expire_common.c create mode 100644 src/tests/cmocka/test_expire_common.h create mode 100644 src/tests/cmocka/test_ldap_auth.c create mode 100644 src/tests/cmocka/test_sdap_access.c
diff --git a/Makefile.am b/Makefile.am index ba24343c3a2be9edc94ec817d3709a0c78599847..bc37a04fc44d0f8f94abc050fb2c9eff15a65ab6 100644 --- a/Makefile.am +++ b/Makefile.am @@ -212,6 +212,8 @@ if HAVE_CMOCKA sbus-internal-tests \ sss_sifp-tests \ test_search_bases \
test_ldap_auth \
test_sdap_access \ sdap-tests \ test_sysdb_views \ test_be_ptask \
@@ -1970,6 +1972,48 @@ test_search_bases_LDADD = \ libsss_krb5_common.la \ libsss_test_common.la
+test_ldap_auth_SOURCES = \
- $(sssd_be_SOURCES) \
- src/tests/cmocka/test_ldap_auth.c \
- src/tests/cmocka/test_expire_common.c \
- $(NULL)
+test_ldap_auth_CFLAGS = \
- $(AM_CFLAGS) \
- -DUNIT_TESTING
+test_ldap_auth_LDADD = \
- $(PAM_LIBS) \
- $(CMOCKA_LIBS) \
- $(POPT_LIBS) \
- $(SSSD_LIBS) \
- $(CARES_LIBS) \
- $(KRB5_LIBS) \
- $(SSSD_INTERNAL_LTLIBS) \
- libsss_ldap_common.la \
- libsss_test_common.la \
- $(NULL)
+test_sdap_access_SOURCES = \
- $(sssd_be_SOURCES) \
- src/tests/cmocka/test_sdap_access.c \
- src/tests/cmocka/test_expire_common.c \
- $(NULL)
+test_sdap_access_CFLAGS = \
- $(AM_CFLAGS) \
- -DUNIT_TESTING
+test_sdap_access_LDADD = \
- $(PAM_LIBS) \
- $(CMOCKA_LIBS) \
- $(POPT_LIBS) \
- $(SSSD_LIBS) \
- $(CARES_LIBS) \
- $(KRB5_LIBS) \
- $(SSSD_INTERNAL_LTLIBS) \
- libsss_ldap_common.la \
- libsss_test_common.la \
- $(NULL)
I wonder why didn't you create a single pwexpire test instead? The file names are confusing, I would expect tests of authentication and acess control.
+int expire_test_setup(void **state) +{
- struct expire_test_ctx *exp_state;
- TALLOC_CTX *mem_ctx;
- char *past_time;
- char *future_time;
- char *invalid_format;
- char *invalid_longer_format;
- mem_ctx = talloc_new(NULL);
- assert_non_null(mem_ctx);
- exp_state = talloc(mem_ctx, struct expire_test_ctx);
- assert_non_null(exp_state);
- exp_state->mem_ctx = mem_ctx;
Please don't use a memory context as a structure member, use the structure itself.
- *state = exp_state;
- /* testing data */
- invalid_format = (void*)now_str(mem_ctx, "%Y%m%d%H%M%S", -20);
- assert_non_null(invalid_format);
- invalid_longer_format = (void*)now_str(mem_ctx, "%Y%m%d%H%M%SZA", -20);
- assert_non_null(invalid_longer_format);
- past_time = (void*)now_str(mem_ctx, "%Y%m%d%H%M%SZ", -20);
- assert_non_null(past_time);
- future_time = (void*)now_str(mem_ctx, "%Y%m%d%H%M%SZ", 20);
- assert_non_null(future_time);
- exp_state->past_time = past_time;
- exp_state->future_time = future_time;
- exp_state->invalid_format = invalid_format;
- exp_state->invalid_longer_format = invalid_longer_format;
- return 0;
+}
+int expire_test_teardown(void **state) +{
- struct expire_test_ctx *test_ctx;
- test_ctx = talloc_get_type(*state, struct expire_test_ctx);
- assert_non_null(test_ctx);
- talloc_free(test_ctx->mem_ctx);
Just free the context here.
[...]
+struct indata {
- enum pwexpire type;
- void *time_fmt;
+};
+static void f(void *in, void *_out) +{
- errno_t ret;
- struct indata *data = (struct indata*) in;
- ret = check_pwexpire_policy(data->type, data->time_fmt, NULL, 0);
- *(errno_t*)_out = ret;
+}
I know the tests are usually a punk rock area but let's use better names than "f" and "indata", please.
From ea7a5b02856b852d89efacf33c2bcf2332036901 Mon Sep 17 00:00:00 2001 From: Pavel Reichl preichl@redhat.com Date: Mon, 16 Mar 2015 06:38:43 -0400 Subject: [PATCH 2/3] ldap: refactor check_pwexpire_kerberos to use util func
ACK
From 43cf5bcaadbaedea3280462b736c78a0744b3e9e Mon Sep 17 00:00:00 2001 From: Pavel Reichl preichl@redhat.com Date: Mon, 16 Mar 2015 06:51:37 -0400 Subject: [PATCH 3/3] ldap: refactor nds_check_expired to use util func
ACK
On 03/20/2015 12:11 PM, Jakub Hrozek wrote:
On Mon, Mar 16, 2015 at 12:10:02PM +0100, Pavel Reichl wrote:
Hello,
I recently introduced new utility function sss_utc_to_time_t(). This patches should refactor some existing code to reuse it.
1st patch - add unit tests for fuctions to be amended. 2nd patch - amends check_pwexpire_kerberos() and modifies test - to clearly show how has behavior changed. 3rd patch - amends nds_check_expired() - no test changes required
ci of this version of patches has passed. http://sssd-ci.duckdns.org/logs/job/10/95/summary.html
Thanks!
The refactoring looks good to me but I don't like the tests :-)
From c323db03ff2c72dbfe6496babe2c0920a412118c Mon Sep 17 00:00:00 2001 From: Pavel Reichl preichl@redhat.com Date: Fri, 6 Mar 2015 04:29:24 -0500 Subject: [PATCH 1/3] TESTS: test expiration
Makefile.am | 44 ++++++++++++ src/providers/ldap/sdap_access.c | 2 +- src/tests/cmocka/test_expire_common.c | 130 ++++++++++++++++++++++++++++++++++ src/tests/cmocka/test_expire_common.h | 40 +++++++++++ src/tests/cmocka/test_ldap_auth.c | 99 ++++++++++++++++++++++++++ src/tests/cmocka/test_sdap_access.c | 71 +++++++++++++++++++ 6 files changed, 385 insertions(+), 1 deletion(-) create mode 100644 src/tests/cmocka/test_expire_common.c create mode 100644 src/tests/cmocka/test_expire_common.h create mode 100644 src/tests/cmocka/test_ldap_auth.c create mode 100644 src/tests/cmocka/test_sdap_access.c
diff --git a/Makefile.am b/Makefile.am index ba24343c3a2be9edc94ec817d3709a0c78599847..bc37a04fc44d0f8f94abc050fb2c9eff15a65ab6 100644 --- a/Makefile.am +++ b/Makefile.am @@ -212,6 +212,8 @@ if HAVE_CMOCKA sbus-internal-tests \ sss_sifp-tests \ test_search_bases \
test_ldap_auth \
test_sdap_access \ sdap-tests \ test_sysdb_views \ test_be_ptask \
@@ -1970,6 +1972,48 @@ test_search_bases_LDADD = \ libsss_krb5_common.la \ libsss_test_common.la
+test_ldap_auth_SOURCES = \
- $(sssd_be_SOURCES) \
- src/tests/cmocka/test_ldap_auth.c \
- src/tests/cmocka/test_expire_common.c \
- $(NULL)
+test_ldap_auth_CFLAGS = \
- $(AM_CFLAGS) \
- -DUNIT_TESTING
+test_ldap_auth_LDADD = \
- $(PAM_LIBS) \
- $(CMOCKA_LIBS) \
- $(POPT_LIBS) \
- $(SSSD_LIBS) \
- $(CARES_LIBS) \
- $(KRB5_LIBS) \
- $(SSSD_INTERNAL_LTLIBS) \
- libsss_ldap_common.la \
- libsss_test_common.la \
- $(NULL)
+test_sdap_access_SOURCES = \
- $(sssd_be_SOURCES) \
- src/tests/cmocka/test_sdap_access.c \
- src/tests/cmocka/test_expire_common.c \
- $(NULL)
+test_sdap_access_CFLAGS = \
- $(AM_CFLAGS) \
- -DUNIT_TESTING
+test_sdap_access_LDADD = \
- $(PAM_LIBS) \
- $(CMOCKA_LIBS) \
- $(POPT_LIBS) \
- $(SSSD_LIBS) \
- $(CARES_LIBS) \
- $(KRB5_LIBS) \
- $(SSSD_INTERNAL_LTLIBS) \
- libsss_ldap_common.la \
- libsss_test_common.la \
- $(NULL)
I wonder why didn't you create a single pwexpire test instead? The file names are confusing, I would expect tests of authentication and acess control.
The functionality I wanted to test was in 2 separate units (ldap_auth,c, sdap_access.c), but for those units there were no tests. So I created 2 new test units (test_ldap_auth,c, test_sdap_access.c) in hope that more functions from these units will be tested. (code coverage is currently 0.0 % for both modules (https://jhrozek.fedorapeople.org/sssd-coverage/html/providers/ldap/index.htm...)
I can merge the tests into one file and just test the functionality that the 3rd patch is changing. I guess this would be more consistent with the way other tests work.
+int expire_test_setup(void **state) +{
- struct expire_test_ctx *exp_state;
- TALLOC_CTX *mem_ctx;
- char *past_time;
- char *future_time;
- char *invalid_format;
- char *invalid_longer_format;
- mem_ctx = talloc_new(NULL);
- assert_non_null(mem_ctx);
- exp_state = talloc(mem_ctx, struct expire_test_ctx);
- assert_non_null(exp_state);
- exp_state->mem_ctx = mem_ctx;
Please don't use a memory context as a structure member, use the structure itself.
- *state = exp_state;
- /* testing data */
- invalid_format = (void*)now_str(mem_ctx, "%Y%m%d%H%M%S", -20);
- assert_non_null(invalid_format);
- invalid_longer_format = (void*)now_str(mem_ctx, "%Y%m%d%H%M%SZA", -20);
- assert_non_null(invalid_longer_format);
- past_time = (void*)now_str(mem_ctx, "%Y%m%d%H%M%SZ", -20);
- assert_non_null(past_time);
- future_time = (void*)now_str(mem_ctx, "%Y%m%d%H%M%SZ", 20);
- assert_non_null(future_time);
- exp_state->past_time = past_time;
- exp_state->future_time = future_time;
- exp_state->invalid_format = invalid_format;
- exp_state->invalid_longer_format = invalid_longer_format;
- return 0;
+}
+int expire_test_teardown(void **state) +{
- struct expire_test_ctx *test_ctx;
- test_ctx = talloc_get_type(*state, struct expire_test_ctx);
- assert_non_null(test_ctx);
- talloc_free(test_ctx->mem_ctx);
Just free the context here.
[...]
+struct indata {
- enum pwexpire type;
- void *time_fmt;
+};
+static void f(void *in, void *_out) +{
- errno_t ret;
- struct indata *data = (struct indata*) in;
- ret = check_pwexpire_policy(data->type, data->time_fmt, NULL, 0);
- *(errno_t*)_out = ret;
+}
I know the tests are usually a punk rock area but let's use better names than "f" and "indata", please.
From ea7a5b02856b852d89efacf33c2bcf2332036901 Mon Sep 17 00:00:00 2001 From: Pavel Reichl preichl@redhat.com Date: Mon, 16 Mar 2015 06:38:43 -0400 Subject: [PATCH 2/3] ldap: refactor check_pwexpire_kerberos to use util func
ACK
From 43cf5bcaadbaedea3280462b736c78a0744b3e9e Mon Sep 17 00:00:00 2001 From: Pavel Reichl preichl@redhat.com Date: Mon, 16 Mar 2015 06:51:37 -0400 Subject: [PATCH 3/3] ldap: refactor nds_check_expired to use util func
ACK _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On Fri, Mar 20, 2015 at 01:13:35PM +0100, Pavel Reichl wrote:
I wonder why didn't you create a single pwexpire test instead? The file names are confusing, I would expect tests of authentication and acess control.
The functionality I wanted to test was in 2 separate units (ldap_auth,c, sdap_access.c), but for those units there were no tests. So I created 2 new test units (test_ldap_auth,c, test_sdap_access.c) in hope that more functions from these units will be tested. (code coverage is currently 0.0 % for both modules (https://jhrozek.fedorapeople.org/sssd-coverage/html/providers/ldap/index.htm...)
I think this is OK as long as there's nothing specific to the expiration tests in the test sources and the sources can be re-used for 'real' auth and access checks.
(btw I'm not sure how realistic is to have auth and access /unit/ tests, I looked at the sources when you wrote the SSH key expiration patches, but it seemed to me integration tests could be a better option)
So I'm fine both ways.
On 03/20/2015 03:26 PM, Jakub Hrozek wrote:
On Fri, Mar 20, 2015 at 01:13:35PM +0100, Pavel Reichl wrote:
I wonder why didn't you create a single pwexpire test instead? The file names are confusing, I would expect tests of authentication and acess control.
The functionality I wanted to test was in 2 separate units (ldap_auth,c, sdap_access.c), but for those units there were no tests. So I created 2 new test units (test_ldap_auth,c, test_sdap_access.c) in hope that more functions from these units will be tested. (code coverage is currently 0.0 % for both modules (https://jhrozek.fedorapeople.org/sssd-coverage/html/providers/ldap/index.htm...)
I think this is OK as long as there's nothing specific to the expiration tests in the test sources and the sources can be re-used for 'real' auth and access checks.
(btw I'm not sure how realistic is to have auth and access /unit/ tests, I looked at the sources when you wrote the SSH key expiration patches, but it seemed to me integration tests could be a better option)
So I'm fine both ways. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Please see updated patch set. Thanks!
ci passed:
On Fri, Mar 20, 2015 at 04:58:38PM +0100, Pavel Reichl wrote:
On 03/20/2015 03:26 PM, Jakub Hrozek wrote:
On Fri, Mar 20, 2015 at 01:13:35PM +0100, Pavel Reichl wrote:
I wonder why didn't you create a single pwexpire test instead? The file names are confusing, I would expect tests of authentication and acess control.
The functionality I wanted to test was in 2 separate units (ldap_auth,c, sdap_access.c), but for those units there were no tests. So I created 2 new test units (test_ldap_auth,c, test_sdap_access.c) in hope that more functions from these units will be tested. (code coverage is currently 0.0 % for both modules (https://jhrozek.fedorapeople.org/sssd-coverage/html/providers/ldap/index.htm...)
I think this is OK as long as there's nothing specific to the expiration tests in the test sources and the sources can be re-used for 'real' auth and access checks.
(btw I'm not sure how realistic is to have auth and access /unit/ tests, I looked at the sources when you wrote the SSH key expiration patches, but it seemed to me integration tests could be a better option)
So I'm fine both ways. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Please see updated patch set. Thanks!
ci passed:
From 4236caecdc9981d10f777e90a65aa60cd92445df Mon Sep 17 00:00:00 2001 From: Pavel Reichl preichl@redhat.com Date: Fri, 6 Mar 2015 04:29:24 -0500 Subject: [PATCH 1/3] TESTS: test expiration
Sorry, more nitpicks. I should have sent them earlier, but I didn't notice them before.
Makefile.am | 44 ++++++++++++ src/providers/ldap/sdap_access.c | 2 +- src/tests/cmocka/test_expire_common.c | 130 ++++++++++++++++++++++++++++++++++ src/tests/cmocka/test_expire_common.h | 39 ++++++++++ src/tests/cmocka/test_ldap_auth.c | 106 +++++++++++++++++++++++++++ src/tests/cmocka/test_sdap_access.c | 73 +++++++++++++++++++ 6 files changed, 393 insertions(+), 1 deletion(-) create mode 100644 src/tests/cmocka/test_expire_common.c create mode 100644 src/tests/cmocka/test_expire_common.h create mode 100644 src/tests/cmocka/test_ldap_auth.c create mode 100644 src/tests/cmocka/test_sdap_access.c
diff --git a/Makefile.am b/Makefile.am index 0f7725b06e0330bf17c38eb7b1829a8093fb1d70..6f787e625fbd67d55e953f374bd435f0bbcf3d76 100644 --- a/Makefile.am +++ b/Makefile.am @@ -212,6 +212,8 @@ if HAVE_CMOCKA sbus-internal-tests \ sss_sifp-tests \ test_search_bases \
test_ldap_auth \
test_sdap_access \ sdap-tests \ test_sysdb_views \ test_sysdb_utils \
@@ -1971,6 +1973,48 @@ test_search_bases_LDADD = \ libsss_krb5_common.la \ libsss_test_common.la
+test_ldap_auth_SOURCES = \
- $(sssd_be_SOURCES) \
- src/tests/cmocka/test_ldap_auth.c \
- src/tests/cmocka/test_expire_common.c \
- $(NULL)
I prefer not to have an extra line between _SOURCES and _CFLAGS for the same target.
+test_ldap_auth_CFLAGS = \
- $(AM_CFLAGS) \
- -DUNIT_TESTING
Please add $(NULL) here.
+test_ldap_auth_LDADD = \
- $(PAM_LIBS) \
- $(CMOCKA_LIBS) \
- $(POPT_LIBS) \
- $(SSSD_LIBS) \
- $(CARES_LIBS) \
- $(KRB5_LIBS) \
- $(SSSD_INTERNAL_LTLIBS) \
- libsss_ldap_common.la \
- libsss_test_common.la \
- $(NULL)
+test_sdap_access_SOURCES = \
- $(sssd_be_SOURCES) \
- src/tests/cmocka/test_sdap_access.c \
- src/tests/cmocka/test_expire_common.c \
- $(NULL)
Extra blank line
+test_sdap_access_CFLAGS = \
- $(AM_CFLAGS) \
- -DUNIT_TESTING
Missing $(NULL)
+test_sdap_access_LDADD = \
- $(PAM_LIBS) \
- $(CMOCKA_LIBS) \
- $(POPT_LIBS) \
- $(SSSD_LIBS) \
- $(CARES_LIBS) \
- $(KRB5_LIBS) \
- $(SSSD_INTERNAL_LTLIBS) \
- libsss_ldap_common.la \
- libsss_test_common.la \
- $(NULL)
ad_access_filter_tests_SOURCES = \ $(sssd_be_SOURCES) \ src/providers/ad/ad_common.c \ diff --git a/src/providers/ldap/sdap_access.c b/src/providers/ldap/sdap_access.c index be2dacf5379a6842c841fe77fb61aa4ebea03e1f..9e9cfc9e3e6d6f0ef25fdd5eb962c5f42e24b60f 100644 --- a/src/providers/ldap/sdap_access.c +++ b/src/providers/ldap/sdap_access.c @@ -528,7 +528,7 @@ static errno_t sdap_account_expired_rhds(struct pam_data *pd, #define NDS_EXPIRED_MSG "The user account is expired" #define NDS_TIME_MAP_MSG "The user account is not allowed at this time"
-static bool nds_check_expired(const char *exp_time_str) +bool nds_check_expired(const char *exp_time_str) { char *end; struct tm tm; diff --git a/src/tests/cmocka/test_expire_common.c b/src/tests/cmocka/test_expire_common.c new file mode 100644 index 0000000000000000000000000000000000000000..73a3b23f50c3935b6cbb3320d50589f626445e3d --- /dev/null +++ b/src/tests/cmocka/test_expire_common.c @@ -0,0 +1,130 @@ +/*
- Authors:
Pavel Reichl <preichl@redhat.com>
- Copyright (C) 2015 Red Hat
- SSSD tests - common code for password expiration tests
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 3 of the License, or
- (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program. If not, see http://www.gnu.org/licenses/.
+*/
+#include <stdarg.h> +#include <stdlib.h> +#include <stddef.h> +#include <setjmp.h> +#include <unistd.h> +#include <sys/types.h> +#include <cmocka.h> +#include <time.h>
+#include "tests/common_check.h" +#include "tests/cmocka/test_expire_common.h"
+#define MAX 100
+static char *now_str(TALLOC_CTX *mem_ctx, const char* format, int s) +{
- time_t t = time(NULL) + s;
- struct tm *tm;
- size_t len;
- char *timestr;
- timestr = talloc_array(mem_ctx, char, MAX);
- tm = gmtime(&t);
- len = strftime(timestr, MAX, format, tm);
- if (len == 0) {
return NULL;
- }
- return timestr;
+}
+int expire_test_setup(void **state) +{
- struct expire_test_ctx *exp_state;
- TALLOC_CTX *mem_ctx;
- char *past_time;
- char *future_time;
- char *invalid_format;
- char *invalid_longer_format;
- mem_ctx = talloc_new(NULL);
- assert_non_null(mem_ctx);
- exp_state = talloc(mem_ctx, struct expire_test_ctx);
- assert_non_null(exp_state);
- *state = exp_state;
- /* testing data */
- invalid_format = (void*)now_str(exp_state, "%Y%m%d%H%M%S", -20);
^^^^^^ I don't think you need the void* casts, can you just drop them?
- assert_non_null(invalid_format);
- invalid_longer_format = (void*)now_str(exp_state, "%Y%m%d%H%M%SZA", -20);
- assert_non_null(invalid_longer_format);
- past_time = (void*)now_str(exp_state, "%Y%m%d%H%M%SZ", -20);
- assert_non_null(past_time);
- future_time = (void*)now_str(exp_state, "%Y%m%d%H%M%SZ", 20);
- assert_non_null(future_time);
- exp_state->past_time = past_time;
- exp_state->future_time = future_time;
- exp_state->invalid_format = invalid_format;
- exp_state->invalid_longer_format = invalid_longer_format;
- return 0;
+}
+int expire_test_teardown(void **state) +{
- struct expire_test_ctx *test_ctx;
- test_ctx = talloc_get_type(*state, struct expire_test_ctx);
- assert_non_null(test_ctx);
- talloc_free(test_ctx);
- return 0;
+}
+void expire_test_tz(const char* tz,
void (*test_func)(void*, void*),
void *test_in,
void *_test_out)
+{
- errno_t ret;
- const char *orig_tz = NULL;
- orig_tz = getenv("TZ");
- if (orig_tz == NULL) {
orig_tz = "";
- }
- if (tz) {
ret = setenv("TZ", tz, 1);
assert_false(ret == -1);
- }
- test_func(test_in, _test_out);
- /* restore */
- if (orig_tz != NULL) {
ret = setenv("TZ", orig_tz, 1);
assert_false(ret == -1);
- }
+} diff --git a/src/tests/cmocka/test_expire_common.h b/src/tests/cmocka/test_expire_common.h new file mode 100644 index 0000000000000000000000000000000000000000..0ccff14fc90aafc85915f8e6b2087d5ed09fa84a --- /dev/null +++ b/src/tests/cmocka/test_expire_common.h @@ -0,0 +1,39 @@ +/*
- Authors:
Pavel Reichl <preichl@redhat.com>
- Copyright (C) 2015 Red Hat
- SSSD tests: Tests for password expiration related functionality
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 3 of the License, or
- (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program. If not, see http://www.gnu.org/licenses/.
+*/
+#ifndef __TEST_EXPIRE_COMMON_H +#define __TEST_EXPIRE_COMMON_H
+struct expire_test_ctx +{
- char *past_time;
- char *future_time;
- char *invalid_format;
- char *invalid_longer_format;
+};
+int expire_test_setup(void **state); +int expire_test_teardown(void **state); +void expire_test_tz(const char* tz, void (*f)(void*, void*), void *in,
void *_out);
+#endif /* __TEST_EXPIRE_COMMON_H */ diff --git a/src/tests/cmocka/test_ldap_auth.c b/src/tests/cmocka/test_ldap_auth.c new file mode 100644 index 0000000000000000000000000000000000000000..00bb3514e6826406b844e42f569a0ee657808252 --- /dev/null +++ b/src/tests/cmocka/test_ldap_auth.c @@ -0,0 +1,106 @@ +/*
- Authors:
Pavel Reichl <preichl@redhat.com>
- Copyright (C) 2015 Red Hat
- SSSD tests - ldap auth
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 3 of the License, or
- (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program. If not, see http://www.gnu.org/licenses/.
+*/
+#include <stdarg.h> +#include <stdlib.h> +#include <stddef.h> +#include <setjmp.h> +#include <unistd.h> +#include <sys/types.h> +#include <cmocka.h>
+#include "tests/common_check.h" +#include "providers/ldap/ldap_auth.h" +#include "tests/cmocka/test_expire_common.h"
+struct check_pwexpire_policy_wrap_indata {
- enum pwexpire type;
- void *time_fmt;
+};
+static void check_pwexpire_policy_wrap(void *in, void *_out) +{
- errno_t ret;
- struct check_pwexpire_policy_wrap_indata *data =
(struct check_pwexpire_policy_wrap_indata*) in;
- ret = check_pwexpire_policy(data->type, data->time_fmt, NULL, 0);
- *(errno_t*)_out = ret;
+}
+static void test(void **state, enum pwexpire type) +{
- struct expire_test_ctx *tc;
- errno_t ret;
- tc = talloc_get_type(*state, struct expire_test_ctx);
- assert_non_null(tc);
- ret = check_pwexpire_policy(type,
(void*) tc->invalid_longer_format, NULL, 0);
- assert_true(ret == EINVAL);
- ret = check_pwexpire_policy(type, (void*) tc->invalid_format,
NULL, 0);
- assert_true(ret == EINVAL);
- ret = check_pwexpire_policy(type, (void*) tc->past_time,
NULL, 0);
- assert_true(ret == ERR_PASSWORD_EXPIRED);
- ret = check_pwexpire_policy(type, (void*) tc->future_time,
NULL, 0);
- assert_true(ret == EOK);
- /* changing time zone has no effect as time of expiration is in UTC */
- struct check_pwexpire_policy_wrap_indata data;
- data.type = type;
- data.time_fmt = (void*)tc->future_time;
- expire_test_tz("GST-2",
check_pwexpire_policy_wrap,
(void*)&data,
(void*)&ret);
- assert_true(ret == EOK);
- data.time_fmt = (void*)tc->past_time;
- expire_test_tz("GST-2",
check_pwexpire_policy_wrap,
(void*)&data,
(void*)&ret);
- assert_true(ret == ERR_PASSWORD_EXPIRED);
+}
+void test_pwexpire_krb(void **state) +{
- test(state, PWEXPIRE_KERBEROS);
Test is a static function that is only called once, does it need to be a separate function?
+}
+int main(void) +{
- const struct CMUnitTest tests[] = {
cmocka_unit_test_setup_teardown(test_pwexpire_krb,
expire_test_setup,
expire_test_teardown)
- };
- return cmocka_run_group_tests(tests, NULL, NULL);
+} diff --git a/src/tests/cmocka/test_sdap_access.c b/src/tests/cmocka/test_sdap_access.c new file mode 100644 index 0000000000000000000000000000000000000000..6387e54ac092523dfd4407983525837c229d4772 --- /dev/null +++ b/src/tests/cmocka/test_sdap_access.c @@ -0,0 +1,73 @@ +/*
- Authors:
Pavel Reichl <preichl@redhat.com>
- Copyright (C) 2015 Red Hat
- SSSD tests - sdap access
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 3 of the License, or
- (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program. If not, see http://www.gnu.org/licenses/.
+*/
+#include <stdarg.h> +#include <stdlib.h> +#include <stddef.h> +#include <setjmp.h> +#include <unistd.h> +#include <sys/types.h> +#include <cmocka.h>
+#include "tests/common_check.h" +#include "tests/cmocka/test_expire_common.h"
+bool nds_check_expired(const char *exp_time_str);
Can you add "extern" here or at least a comment that we're linking with a function from an sdap module?
+static void nds_check_expired_wrap(void *in, void *_out) +{
- *(bool*)_out = nds_check_expired((const char*)in);
+}
+void test_nds_check_expire(void **state) +{
- struct expire_test_ctx *tc;
- bool res;
- tc = talloc_get_type(*state, struct expire_test_ctx);
- assert_non_null(tc);
- assert_false(nds_check_expired(NULL));
- assert_true(nds_check_expired(tc->invalid_longer_format));
- assert_true(nds_check_expired(tc->invalid_format));
- assert_true(nds_check_expired(tc->past_time));
- assert_false(nds_check_expired(tc->future_time));
- /* changing time zone has no effect as time of expiration is in UTC */
- expire_test_tz("GST+2", nds_check_expired_wrap, (void*)tc->future_time,
(void*)&res);
- assert_false(res);
- expire_test_tz("GST-2", nds_check_expired_wrap, (void*)tc->future_time,
(void*)&res);
- assert_false(res);
+}
+int main(void) +{
- const struct CMUnitTest tests[] = {
cmocka_unit_test_setup_teardown(test_nds_check_expire,
expire_test_setup,
expire_test_teardown)
- };
- return cmocka_run_group_tests(tests, NULL, NULL);
+}
2.1.0
From 546e0f40f8326f733133dac842b1335f0f1bd6c8 Mon Sep 17 00:00:00 2001 From: Pavel Reichl preichl@redhat.com Date: Mon, 16 Mar 2015 06:38:43 -0400 Subject: [PATCH 2/3] ldap: refactor check_pwexpire_kerberos to use util func
still ack
From 7abde9e3a85c1ca6f9cfe1a26bbb9c42a37ab3d6 Mon Sep 17 00:00:00 2001 From: Pavel Reichl preichl@redhat.com Date: Mon, 16 Mar 2015 06:51:37 -0400 Subject: [PATCH 3/3] ldap: refactor nds_check_expired to use util func
still ack
On 03/20/2015 06:44 PM, Jakub Hrozek wrote:
On Fri, Mar 20, 2015 at 04:58:38PM +0100, Pavel Reichl wrote:
On 03/20/2015 03:26 PM, Jakub Hrozek wrote:
On Fri, Mar 20, 2015 at 01:13:35PM +0100, Pavel Reichl wrote:
I wonder why didn't you create a single pwexpire test instead? The file names are confusing, I would expect tests of authentication and acess control.
The functionality I wanted to test was in 2 separate units (ldap_auth,c, sdap_access.c), but for those units there were no tests. So I created 2 new test units (test_ldap_auth,c, test_sdap_access.c) in hope that more functions from these units will be tested. (code coverage is currently 0.0 % for both modules (https://jhrozek.fedorapeople.org/sssd-coverage/html/providers/ldap/index.htm...)
I think this is OK as long as there's nothing specific to the expiration tests in the test sources and the sources can be re-used for 'real' auth and access checks.
(btw I'm not sure how realistic is to have auth and access /unit/ tests, I looked at the sources when you wrote the SSH key expiration patches, but it seemed to me integration tests could be a better option)
So I'm fine both ways. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Please see updated patch set. Thanks!
ci passed:
http://sssd-ci.duckdns.org/logs/job/11/31/summary.html
From 4236caecdc9981d10f777e90a65aa60cd92445df Mon Sep 17 00:00:00 2001 From: Pavel Reichl preichl@redhat.com Date: Fri, 6 Mar 2015 04:29:24 -0500 Subject: [PATCH 1/3] TESTS: test expiration
Sorry, more nitpicks. I should have sent them earlier, but I didn't notice them before.
np, please see updated patch set.
Thanks.
On Mon, Mar 23, 2015 at 10:48:59AM +0100, Pavel Reichl wrote:
From 4236caecdc9981d10f777e90a65aa60cd92445df Mon Sep 17 00:00:00 2001 From: Pavel Reichl preichl@redhat.com Date: Fri, 6 Mar 2015 04:29:24 -0500 Subject: [PATCH 1/3] TESTS: test expiration
Sorry, more nitpicks. I should have sent them earlier, but I didn't notice them before.
np, please see updated patch set.
Thanks.
OK, these work for me.
ACK
On Mon, Mar 23, 2015 at 12:06:12PM +0100, Jakub Hrozek wrote:
On Mon, Mar 23, 2015 at 10:48:59AM +0100, Pavel Reichl wrote:
From 4236caecdc9981d10f777e90a65aa60cd92445df Mon Sep 17 00:00:00 2001 From: Pavel Reichl preichl@redhat.com Date: Fri, 6 Mar 2015 04:29:24 -0500 Subject: [PATCH 1/3] TESTS: test expiration
Sorry, more nitpicks. I should have sent them earlier, but I didn't notice them before.
np, please see updated patch set.
Thanks.
OK, these work for me.
ACK
* 08f83281cf4b0f35e8569851fae7364e140371f9 * 0ec41ab7d3fbb021967de16ea000c69dcedf7cb5 * 50b8a36b0932a510e825ed1ad8103f81ead2b7d8
sssd-devel@lists.fedorahosted.org