On 02/10/2016 12:52 PM, Jakub Hrozek wrote:
On Wed, Feb 10, 2016 at 12:43:18PM +0100, Pavel Reichl wrote:
On 02/10/2016 12:04 PM, Lukas Slebodnik wrote:
On (10/02/16 11:38), Pavel Březina wrote:
On 02/05/2016 05:13 PM, Lukas Slebodnik wrote:
On (05/02/16 16:56), Pavel Reichl wrote:
Hopefully the last one.
>From 58b06b8fee18c4242e301ecf71f92a77f6ba6e7b Mon Sep 17 00:00:00 2001
Hi,
I will chime in here since this mail remains clear...
+#define TEST_2922_MIN_ID 1842600000
+#define TEST_2922_MAX_ID 1842799999
+#define TEST_2922_MAX_ID_PLUS_ONE (TEST_2922_MAX_ID + 1)
+
+#define TEST_2922_DFL_SLIDE 9212
+#define TEST_2922_FIRST_SID TEST_DOM_SID"-0"
+/* Last SID = first SID + (default) rangesize -1 */
+#define TEST_2922_LAST_SID TEST_DOM_SID"-199999"
+/* Last SID = first SID + rangesize */
+#define TEST_2922_LAST_SID_PLUS_ONE TEST_DOM_SID"-200000"
+
We should prefer to use constants instead of #defines.
Constants are more type safe.
I don't really care about type safety here I care about correct description
of magic values and no duplication of them. Given the #defines prefix those
can be used only for this test and some of the macros are used across the
test and fixture I'd prefer combination of both.
Keep MIN_ID and MAX_ID as macros. Drop _PLUS_ONE completely since "MAX_ID +
1" is descriptive enough. The rest should be as static const since they are
relevant only to one function in one test.
Is that good enough compromise?
struct test_ctx {
TALLOC_CTX *mem_idmap;
struct sss_idmap_ctx *idmap_ctx;
@@ -128,6 +139,53 @@ static int setup_ranges(struct test_ctx *test_ctx, bool
external_mapping,
return 0;
}
+static int setup_ranges_2922(struct test_ctx *test_ctx)
+{
+ struct sss_idmap_range range;
+ enum idmap_error_code err;
+ const char *name;
+ const char *sid;
+ /* Pick a new slice. */
+ id_t slice_num = -1;
+
+ if (test_ctx == NULL) {
+ DEBUG(SSSDBG_FATAL_FAILURE,
+ "test context is NULL\n");
+ return 1;
+ }
+
+ name = TEST_DOM_NAME;
+ sid = TEST_DOM_SID;
+
+ err = sss_idmap_calculate_range(test_ctx->idmap_ctx, sid, &slice_num,
+ &range);
+ if (err != IDMAP_SUCCESS) {
+ DEBUG(SSSDBG_FATAL_FAILURE,
+ "sss_idmap_calculate_range failed: exp: [%d] but got [%d]\n",
+ IDMAP_SUCCESS, err);
+ return 1;
Please use different numbers for different failures.
Persoanly, I do not like theusage of such error handling in tests.
But that was a desigh decision of cmocka developers
which I do not consider as the right one.
Old versions of cmocka allowed to use assertions in setup and teradown
function. The same applies to check framework. Such decision add
unnecassary complexity to the review process. Reviewer need to check
that different error codes are returned or debug message is logged
with proper debug level. But that's the separete discusssion.
You can still use assertions in fixtures (I just checked with asn to be sure)
and that is the way we should lean to. So forget error codes and debug
messages here, use assertions to make it cleaner and shorter.
Thank you for another oninion. I prefer assertion as well but I was not
stricly against error codes.
You proposed more/less the same things as I wrote in my last mails.
Well no, asserts were out of question because they were banned by Jakub (2nd
mail in this thread). If Jakub is fine with asserts I'm all for it.
I didn't ban anything, I just don't like the way cmocka errors out with
asserts in fixtures (and I'm probably half-guilty for that..)
Sorry, I meant that I considered them as no way to go - I thought it was based
on discussion you and Michal had.
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
