[SSSD] [PATCH] nested groups unit test

Jakub Hrozek jhrozek at redhat.com
Tue Feb 18 21:49:49 UTC 2014


On Tue, Feb 18, 2014 at 04:55:54PM +0100, Pavel Březina wrote:
> >[PATCH 01/13] sdap: move non async functions from sdap_async.c to sdap_utils.c
> >ACK
> >
> >[PATCH 02/13] sdap: move non async functions from sdap_async_connection.c to sdap_utils.c
> >ACK, just can we napespace deref_string_to_val in the future?
> >
> >[PATCH 03/13] sdap: move sdap_get_id_specific_filter() to sdap_utils.c
> >ACK
> >
> >[PATCH 04/13] ldap: move options related content from ldap_common.c to ldap_options.c
> >ACK
> >
> >[PATCH 05/13] ldap: move domain related content from ldap_common.c to sdap_domain.c
> >ACK
> >
> >[PATCH 06/13] make make_realm_upper_case() static
> >ACK
> >
> >[PATCH 07/13] tests: add confdb_path to sss_test_ctx
> >ACK
> >
> >[PATCH 08/13] tests: mock SDAP
> >Shouldn't the file be called mock_sdap_async.c given that most if
> >not all functions the module mocks come from sdap_async.c ? But ACK to
> >the content of the files.
> 
> It contains everything that is necessary to test sdap modules - all
> asynchronous functions (at this moment only those that are needed
> for this test), options, sdap handle. So I think current name is
> more describing, but I don't really oppose. Tell me if you want me
> to rename it.

Fair enough, the name is not that important after all, we can change it
later if needed.

> 
> >You need to add common_mock_sdap.h to noinst_HEADERS to avoid breaking
> >distcheck.
> 
> Done.

ACK

> 
> >[PATCH 09/13] tests: mock sysdb users and groups
> >I think the shortcut is acceptable in tests as it makes the unit test
> >itself easier to read. Also the ldb values are ultimately just strings,
> >so I don't think the danger here is too big.
> >
> >You need to add common_mock_sysdb_objects.h to noinst_HEADERS to
> >avoid breaking distcheck.
> >
> >mock_sysdb_user() is unused, do you plan on using it in future?
> 
> Yes, it will be used in other nested groups tests.

OK, let's keep it, then,

ACK

> 
> >[PATCH 10/13] tests: prepare makefile for provider related unit
> >ACK. I just verified that SSSD builds fine when this patch is the tip of
> >the tree to make sure we don't break bisect.
> >
> >[PATCH 11/13] tests: new macro sss_will_return_always
> >ACK, same simple test as above
> >
> >[PATCH 12/13] tests: nested groups unit test
> >ACK. Do you plan on adding more tests in the future with different group
> >nestings etc? Then even mock_sysdb_user could be useful..
> 
> Of course. As it is mentioned in the first e-mail, I wanted to make
> sure that we agree on this framework before I spend more time on
> writing tests.

Great. I admit I don't know what was in the first e-mail anymore since
the patches were idling on the list for so long.

ACK

> 
> >[PATCH 13/13] tests: don't print debug message when test dir does
> >ACK
> 
> Thank you for the review.
> 

All patches look good now. I'll push the whole batch to master. Thank
you for the patience and the time you took to write unit tests.



More information about the sssd-devel mailing list