Re: [SSSD] [PATCH] TESTS: Removing part of responder_cache_req-tests
On Thu, Aug 27, 2015 at 01:05:00PM +0200, Lukas Slebodnik wrote: > On (24/08/15 16:09), Michal Židek wrote: > >On 08/21/2015 04:55 PM, Petr Cech wrote: > >>On 08/21/2015 02:35 PM, Michal Židek wrote: > >>>Hi, > >>> > >>>some of the tests you deleted are valid and > >>>should not be deleted. > >>> > >>>Only those tests that rely on time(NULL) > >>>being the same as the time of request > >>>creation are invalid. All those that test old > >>>entries or nonexistent entries are OK. > >>>See comments inline. > >>I agree. > >>Those tests have another logic. So I returned them back. > >>Petr > >> > > > >ACK. > > > >CI results: > >http://sssd-ci.duckdns.org/logs/job/23/75/summary.html > > > +1 for patch. > > but please do not close the ticket #2730 OK, I changed "Resolves" to "Works Around" in the commit message and pushed the patch to master: bdf422fde0fd6b40b3412bad3b200f8fd7ea8693 > after pushing to master. This solution is just a temporary > workaround for problematic tests. What do you think would be the solution? I thought Michal and Petr concluded that the tests were simply invalid.. ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] TESTS: Removing part of responder_cache_req-tests
On 08/31/2015 06:29 PM, Jakub Hrozek wrote: On Thu, Aug 27, 2015 at 01:05:00PM +0200, Lukas Slebodnik wrote: On (24/08/15 16:09), Michal Židek wrote: On 08/21/2015 04:55 PM, Petr Cech wrote: On 08/21/2015 02:35 PM, Michal Židek wrote: Hi, some of the tests you deleted are valid and should not be deleted. Only those tests that rely on time(NULL) being the same as the time of request creation are invalid. All those that test old entries or nonexistent entries are OK. See comments inline. I agree. Those tests have another logic. So I returned them back. Petr ACK. CI results: http://sssd-ci.duckdns.org/logs/job/23/75/summary.html +1 for patch. but please do not close the ticket #2730 OK, I changed "Resolves" to "Works Around" in the commit message and pushed the patch to master: bdf422fde0fd6b40b3412bad3b200f8fd7ea8693 after pushing to master. This solution is just a temporary workaround for problematic tests. What do you think would be the solution? I thought Michal and Petr concluded that the tests were simply invalid.. The solution is to replace them with valid test. Currently we do not have properly covered some of the functionality in tests (we though it is covered, but the tests were just not valid and did not reflect how the functions are used). Michal ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] TESTS: Removing part of responder_cache_req-tests
On (24/08/15 16:09), Michal Židek wrote: On 08/21/2015 04:55 PM, Petr Cech wrote: On 08/21/2015 02:35 PM, Michal Židek wrote: Hi, some of the tests you deleted are valid and should not be deleted. Only those tests that rely on time(NULL) being the same as the time of request creation are invalid. All those that test old entries or nonexistent entries are OK. See comments inline. I agree. Those tests have another logic. So I returned them back. Petr ACK. CI results: http://sssd-ci.duckdns.org/logs/job/23/75/summary.html +1 for patch. but please do not close the ticket #2730 after pushing to master. This solution is just a temporary workaround for problematic tests. LS ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] TESTS: Removing part of responder_cache_req-tests
On 08/21/2015 04:55 PM, Petr Cech wrote: On 08/21/2015 02:35 PM, Michal Židek wrote: Hi, some of the tests you deleted are valid and should not be deleted. Only those tests that rely on time(NULL) being the same as the time of request creation are invalid. All those that test old entries or nonexistent entries are OK. See comments inline. I agree. Those tests have another logic. So I returned them back. Petr ACK. CI results: http://sssd-ci.duckdns.org/logs/job/23/75/summary.html Michal ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] TESTS: Removing part of responder_cache_req-tests
On 08/21/2015 02:35 PM, Michal Židek wrote:
Hi,
some of the tests you deleted are valid and
should not be deleted.
Only those tests that rely on time(NULL)
being the same as the time of request
creation are invalid. All those that test old
entries or nonexistent entries are OK.
See comments inline.
I agree.
Those tests have another logic. So I returned them back.
Petr
From 63defe03797a8a9038e49400089a732bd35efaca Mon Sep 17 00:00:00 2001
From: Petr Cech pc...@redhat.com
Date: Fri, 21 Aug 2015 16:44:37 +0200
Subject: [PATCH] TESTS: Removing part of responder_cache_req-tests
If you call cache_req_[user|group]_by_filter_send() it than later calls
updated_[users|groups]_by_filter(), which adds filter that is called
recent. This filter causes that only [users|groups] added after the
request started are returned.
This patch removes tests which use
cache_req_[user|group]_by_filter_send(), because the logic of those
tests is corrupted. The tests create [users|groups] and after it, they
call cache_req_[user|group]_by_filter_send(). So it is obvious that it
is not in the right manner.
Possible fix is rewrite the tests to create the entries in the callback.
Resolves:
https://fedorahosted.org/sssd/ticket/2730
---
src/tests/cmocka/test_responder_cache_req.c | 211 ---
1 files changed, 0 insertions(+), 211 deletions(-)
diff --git a/src/tests/cmocka/test_responder_cache_req.c b/src/tests/cmocka/test_responder_cache_req.c
index 032fe429ac88b8cc9113976329ea04837f287276..bc6e2dc8f86a8fa8dc322da10fff4883f075ec7d 100644
--- a/src/tests/cmocka/test_responder_cache_req.c
+++ b/src/tests/cmocka/test_responder_cache_req.c
@@ -1710,54 +1710,6 @@ static void cache_req_user_by_filter_test_done(struct tevent_req *req)
ctx-tctx-done = true;
}
-void test_users_by_filter_valid(void **state)
-{
-struct cache_req_test_ctx *test_ctx = NULL;
-TALLOC_CTX *req_mem_ctx = NULL;
-struct tevent_req *req = NULL;
-const char *ldbname = NULL;
-errno_t ret;
-
-test_ctx = talloc_get_type_abort(*state, struct cache_req_test_ctx);
-test_ctx-create_user = true;
-
-ret = sysdb_store_user(test_ctx-tctx-dom, TEST_USER_NAME2, pwd, 1001, 1001,
- NULL, NULL, NULL, cn=TEST_USER_NAME2,dc=test, NULL,
- NULL, 1000, time(NULL));
-assert_int_equal(ret, EOK);
-
-req_mem_ctx = talloc_new(global_talloc_context);
-check_leaks_push(req_mem_ctx);
-
-/* Filters always go to DP */
-will_return(__wrap_sss_dp_get_account_send, test_ctx);
-mock_account_recv_simple();
-
-req = cache_req_user_by_filter_send(req_mem_ctx, test_ctx-tctx-ev,
-test_ctx-rctx,
-test_ctx-tctx-dom-name,
-test*);
-assert_non_null(req);
-tevent_req_set_callback(req, cache_req_user_by_filter_test_done, test_ctx);
-
-ret = test_ev_loop(test_ctx-tctx);
-assert_int_equal(ret, ERR_OK);
-assert_true(check_leaks_pop(req_mem_ctx));
-
-assert_non_null(test_ctx-result);
-assert_int_equal(test_ctx-result-count, 2);
-
-ldbname = ldb_msg_find_attr_as_string(test_ctx-result-msgs[0],
- SYSDB_NAME, NULL);
-assert_non_null(ldbname);
-assert_string_equal(ldbname, TEST_USER_NAME2);
-
-ldbname = ldb_msg_find_attr_as_string(test_ctx-result-msgs[1],
- SYSDB_NAME, NULL);
-assert_non_null(ldbname);
-assert_string_equal(ldbname, TEST_USER_NAME);
-}
-
void test_users_by_filter_filter_old(void **state)
{
struct cache_req_test_ctx *test_ctx = NULL;
@@ -1831,63 +1783,6 @@ void test_users_by_filter_notfound(void **state)
assert_true(check_leaks_pop(req_mem_ctx));
}
-static void test_users_by_filter_multiple_domains_valid(void **state)
-{
-struct cache_req_test_ctx *test_ctx = NULL;
-struct sss_domain_info *domain = NULL;
-TALLOC_CTX *req_mem_ctx = NULL;
-struct tevent_req *req = NULL;
-const char *ldbname = NULL;
-errno_t ret;
-
-test_ctx = talloc_get_type_abort(*state, struct cache_req_test_ctx);
-
-domain = find_domain_by_name(test_ctx-tctx-dom,
- responder_cache_req_test_d, true);
-assert_non_null(domain);
-
-ret = sysdb_store_user(domain, TEST_USER_NAME, pwd, 1000, 1000,
- NULL, NULL, NULL, cn=TEST_USER_NAME,dc=test, NULL,
- NULL, 1000, time(NULL));
-assert_int_equal(ret, EOK);
-
-ret = sysdb_store_user(domain, TEST_USER_NAME2, pwd, 1001, 1001,
- NULL, NULL, NULL, cn=TEST_USER_NAME2,dc=test, NULL,
- NULL, 1000, time(NULL));
-assert_int_equal(ret, EOK);
-
-req_mem_ctx = talloc_new(global_talloc_context);
-check_leaks_push(req_mem_ctx);
-
-/* Filters always go to DP */
-will_return(__wrap_sss_dp_get_account_send,
