add unit tests to dlck_vector

[osalyk]

Steps for the author:

• Commit message follows the guidelines.
• Appropriate Features or Test-tag pragmas were used.
• Appropriate Functional Test Stages were run.
• At least two positive code reviews including at least one code owner from each category referenced in the PR.
• Testing is complete. If necessary, forced-landing label added and a reason added in a comment.

After all prior steps are complete:

• Gatekeeper requested (daos-gatekeeper added as a reviewer).

---

This change is 

[github-actions]

Errors are component not formatted correctly,Ticket number prefix incorrect,PR title is malformatted. See https://daosio.atlassian.net/wiki/spaces/DC/pages/11133911069/Commit+Comments,Unable to load ticket data
https://daosio.atlassian.net/browse/add

[janekmi]

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 21 unresolved discussions (waiting on @osalyk)

---

a discussion (no related file):
A few more tests I would like to see:

• d_vector_init()
  • think about some tricky entry_size values e.g.
    • entry_size > D_VECTOR_SEGMENT_RAW_CAPACITY
    • D_VECTOR_SEGMENT_RAW_CAPACITY % entry_size != 0
  • d_list_empty(/** ... */) == true
• d_vector_move()
  • move a normally populated vector with an overflow over a single segment.
  • verify the contents of both vectors after the move

---

src/include/daos_srv/d_vector.h line 90 at r1 (raw file):

{
	if (dst == NULL || src == NULL)
		return DER_INVAL;

1. I prefer to keep the if-body in brackets. Just in case.
2. DAOS returns errors as negative values. Hence -DER_INVAL instead of DER_INVAL.

Suggestion:

	if (dst == NULL || src == NULL) {
		return -DER_INVAL;
	}

---

src/include/daos_srv/d_vector.h line 200 at r1 (raw file):

	} else {
		*entry = NULL;
	}

1. Great catch. 🤝 this is why I think good unit tests are essential.
2. I have an unsupported fear this may not be the right place to fix this issue. But I am not sure yet. Can you give it a minute to think whether this is the correct fix? Alternatively I thought maybe the d_vector_segment_entry() function itself should handle this case? What do you think?

Code quote:

	if (load_entry && *idx < (*segment)->dvs_len) {
		*entry = d_vector_segment_entry(*segment, *idx);
	} else {
		*entry = NULL;
	}

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 34 at r1 (raw file):

};

#define ENTRY_SIZE sizeof(struct element)

It is probably unnecessary.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 86 at r1 (raw file):

	struct state *state = *state_ptr;
	int rc = d_vector_append(NULL, &state->array[0]);
	assert_int_equal(rc, DER_INVAL);

Suggestion:

assert_int_equal(rc, -DER_INVAL);

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 94 at r1 (raw file):

	struct state *state = *state_ptr;
	int rc = d_vector_append(&state->vec, NULL);
	assert_int_equal(rc, DER_INVAL);

Suggestion:

assert_int_equal(rc, -DER_INVAL);

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 106 at r1 (raw file):

	d_vector_move(&state->vec, &empty);

	assert_true(d_list_empty(&empty.dv_list));

Probably it makes more sense to use assert_int_equal(d_vector_size(), 0) instead of assert_true(d_list_empty()) just because d_vector_size() is a new interface introduced by this header where d_list_empty() is part an already existing interface.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 116 at r1 (raw file):

	for (int i = 0; i < ARRAY_MAX; ++i)
		d_vector_append(&state->vec, &state->array[i]);

I like to have brackets. I can't help it. 😉

Suggestion:

	for (int i = 0; i < ARRAY_MAX; ++i) {
		d_vector_append(&state->vec, &state->array[i]);
	}

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 118 at r1 (raw file):

		d_vector_append(&state->vec, &state->array[i]);

	d_vector_free(&state->vec);

After the first free. Just as a sanity check. 😅

Code snippet:

assert_true(d_list_empty(&state->vec.dv_list));

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 129 at r1 (raw file):

	struct state *state = *state_ptr;

	int capacity = D_VECTOR_SEGMENT_RAW_CAPACITY / ENTRY_SIZE;

1. I guess having a pointer to vec will shorten this code by a half. 🤣
2. The capacity is already calculated. I think calculating it one more time is confusing.

Suggestion:

d_vector_t *vec = &state->vec;
int capacity = (int)vec->dv_segment_capacity;

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 130 at r1 (raw file):

	int capacity = D_VECTOR_SEGMENT_RAW_CAPACITY / ENTRY_SIZE;

To make sure state->array[capacity] is not out of the boundaries. 👍

Code snippet:

```c
assert_true(capacity + 1 < ARRAY_MAX);

___
*[`src/utils/dlck/tests/dlck_d_vector_ut.c` line 131 at r1](https://reviewable.io/reviews/janekmi/daos/29#-OWWOQbeCQW35EPaKRdY:-OWWOQbeCQW35EPaKRdZ:bp0x5w3) ([raw file](https://github.com/janekmi/daos/blob/f7bd66ea2abcde01a5a66aecd2dcab2eb4fe0f31/src/utils/dlck/tests/dlck_d_vector_ut.c#L131)):*
> ```C
> 	int capacity = D_VECTOR_SEGMENT_RAW_CAPACITY / ENTRY_SIZE;
> 
> 	// Fill the segment completely
> ```

DAOS tends to use the multiline comments format even when the comment is just a single line.
Please apply consistently.



_Suggestion:_
```C
/** Fill the segment completely */

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 139 at r1 (raw file):

	// Add one more item - exceeding capacity
	int rc = d_vector_append(&state->vec, &state->array[capacity]);
	assert_int_equal(rc, DER_SUCCESS);

Suggestion:

	/** Fill the segment completely + one more item - exceeding capacity. */
	for (int i = 0; i <= capacity; i++) {
		int rc = d_vector_append(&state->vec, &state->array[i]);
		assert_int_equal(rc, DER_SUCCESS);
	}

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 142 at r1 (raw file):

	// Check that we have 2 segments
	uint32_t segment_count = 0;

You can for simple counters in this code use either simple types (e.g. int i) or more complex one (e.g. uint32_t segment_count). Just please do it consistently.

Suggestion:

int segment_count = 0;

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 148 at r1 (raw file):

	}
	assert_int_equal(segment_count, 2);

Please verify the contents of the vector.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 149 at r1 (raw file):

	assert_int_equal(segment_count, 2);

	d_vector_free(&state->vec);

• verify the vector is empty after free.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 160 at r1 (raw file):

	uint32_t idx;

	d_vector_init(sizeof(struct element), &state->vec);

It is already done as part of the setup().

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 160 at r1 (raw file):

	uint32_t idx;

	d_vector_init(sizeof(struct element), &state->vec);

Code snippet:

assert_int_equal(d_vector_size(), 0);

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 167 at r1 (raw file):

	}

	d_vector_for_each_entry(entry, segment, idx, &state->vec.dv_list)

This is exactly I do not want to have code without brackets. Do you know how many times the assert two lines below is called? 🤣

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 167 at r1 (raw file):

	}

	d_vector_for_each_entry(entry, segment, idx, &state->vec.dv_list)

Please verify the contents of the vector.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 171 at r1 (raw file):

	assert_int_equal(d_vector_size(&state->vec), ARRAY_MAX);

	d_vector_free(&state->vec);

assert_int_equal(d_vector_size(), 0);

[osalyk]

Reviewable status: 0 of 2 files reviewed, 21 unresolved discussions (waiting on @janekmi)

---

src/include/daos_srv/d_vector.h line 90 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

1. I prefer to keep the if-body in brackets. Just in case.
2. DAOS returns errors as negative values. Hence -DER_INVAL instead of DER_INVAL.

Done.

---

src/include/daos_srv/d_vector.h line 200 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

1. Great catch. 🤝 this is why I think good unit tests are essential.
2. I have an unsupported fear this may not be the right place to fix this issue. But I am not sure yet. Can you give it a minute to think whether this is the correct fix? Alternatively I thought maybe the d_vector_segment_entry() function itself should handle this case? What do you think?

It seems to me that the d_vector_segment_entry() function assumes that we are operating on correctly selected indexes.
My suggestion, let's leave it as it is.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 34 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

It is probably unnecessary.

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 106 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Probably it makes more sense to use assert_int_equal(d_vector_size(), 0) instead of assert_true(d_list_empty()) just because d_vector_size() is a new interface introduced by this header where d_list_empty() is part an already existing interface.

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 116 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I like to have brackets. I can't help it. 😉

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 118 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

After the first free. Just as a sanity check. 😅

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 129 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

1. I guess having a pointer to vec will shorten this code by a half. 🤣
2. The capacity is already calculated. I think calculating it one more time is confusing.

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 130 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

To make sure state->array[capacity] is not out of the boundaries. 👍

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 131 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

DAOS tends to use the multiline comments format even when the comment is just a single line.
Please apply consistently.

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 142 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

You can for simple counters in this code use either simple types (e.g. int i) or more complex one (e.g. uint32_t segment_count). Just please do it consistently.

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 148 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Please verify the contents of the vector.

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 149 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

• verify the vector is empty after free.

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 160 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

It is already done as part of the setup().

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 167 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

This is exactly I do not want to have code without brackets. Do you know how many times the assert two lines below is called? 🤣

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 167 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Please verify the contents of the vector.

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 171 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

assert_int_equal(d_vector_size(), 0);

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 86 at r1 (raw file):

	struct state *state = *state_ptr;
	int rc = d_vector_append(NULL, &state->array[0]);
	assert_int_equal(rc, DER_INVAL);

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 94 at r1 (raw file):

	struct state *state = *state_ptr;
	int rc = d_vector_append(&state->vec, NULL);
	assert_int_equal(rc, DER_INVAL);

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 139 at r1 (raw file):

	// Add one more item - exceeding capacity
	int rc = d_vector_append(&state->vec, &state->array[capacity]);
	assert_int_equal(rc, DER_SUCCESS);

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 160 at r1 (raw file):

	uint32_t idx;

	d_vector_init(sizeof(struct element), &state->vec);

Done.

[janekmi]

Reviewed all commit messages.
Reviewable status: 0 of 2 files reviewed, 4 unresolved discussions (waiting on @osalyk)

---

src/include/daos_srv/d_vector.h line 182 at r1 (raw file):

	bool load_entry = true;

	if (*idx < (*segment)->dvs_len) {

Got it. The issue is here. Not below.

One more time. Despite the proposed solution was not correct. The credit for finding the failing scenario is all yours. Great job! 🤝

Suggestion:

if (*idx + 1 < (*segment)->dvs_len) {

---

src/include/daos_srv/d_vector.h line 200 at r1 (raw file):

	} else {
		*entry = NULL;
	}

This bit was actually correct. Please revert it. Thanks!

Suggestion:

	if (load_entry) {
		*entry = d_vector_segment_entry(*segment, *idx);
	}

[janekmi]

Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @osalyk)

---

src/include/daos_srv/d_vector.h line 201 at r2 (raw file):

	} else {
		*entry = NULL;
	}

I rethought this implementation. I like your idea of segment == NULL as the stop condition for the for-loop. But I would like the logic here as organized as possible. I propose to add a little bit of comments to have a common understanding what each case means.

Three key changes:

1. No more load_entry. If segment != NULL there is an entry to be loaded.
2. idx < segment->dvs_len for the current segment. Always. If we run out of slots a new segment has to be loaded.
3. If we run out of segments:

• segment = NULL and
• entry = NULL and
• idx = 0 just to keep it under control. No plan to use it.

Let's discuss.

Suggestion:

	if (*idx + 1 < (*segment)->dvs_len) { /** there is another entry in the current segment */
		*idx += 1;
	} else { /** look for the next segment */
		d_list_t *next = (*segment)->dvs_link.next;
		if (next != head) { /** the next segment exists */
			(*segment) = d_list_entry(next, __typeof__(**segment), dvs_link);
			prefetch((*segment)->dvs_link.next);
			*idx = 0;
		} else { /** there are no more segments */
			*segment = NULL;
			*idx = 0;
			*entry = NULL;
		}
	}

	if (*segment != NULL) {
		*entry = d_vector_segment_entry(*segment, *idx);
	}

---

src/include/daos_srv/d_vector.h line 208 at r2 (raw file):

         segment != NULL;                                                     \
         _d_vector_foreach_next((void **)&entry, &segment, &idx, head))       \
        if (idx < segment->dvs_len)

We have to discuss this bit offline. I do not understand the purpose of it. 🤔

Code quote:

if (idx < segment->dvs_len)

[janekmi]

Reviewed 2 of 2 files at r2.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @osalyk)

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 105 at r2 (raw file):

	assert_int_equal(d_vector_size(&empty), 0);
	assert_true(d_list_empty(&state->vec.dv_list));

Suggestion:

assert_int_equal(d_vector_size(&state->vec), 0);

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 113 at r2 (raw file):

	struct state *state = *state_ptr;

	for (int i = 0; i < ARRAY_MAX; ++i) {

Just to make sure there are at least two segments to free.

Suggestion:

	struct state *state = *state_ptr;
	
	assert_true(ARRAY_MAX > state->vec.dv_segment_capacity);

	for (int i = 0; i < ARRAY_MAX; ++i) {

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 121 at r2 (raw file):

	d_vector_free(&state->vec);

	assert_true(d_list_empty(&state->vec.dv_list));

1. Spacing:

• operation + check
• newline
• operation + check.

2. d_list_empty -> d_vector_size

Suggestion:

	d_vector_free(&state->vec);
	assert_true(d_vector_size(&state->vec));

	d_vector_free(&state->vec);
	assert_true(d_vector_size(&state->vec));

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 138 at r2 (raw file):

	struct element     *entry;
	d_vector_segment_t *seg;
	uint32_t            idx;

Definitions first. I prefer the order of appearance.

Suggestion:

	struct state *state = *state_ptr;
	d_vector_t *vec = &state->vec;
	int capacity = (int)vec->dv_segment_capacity;
	struct element     *entry;
	d_vector_segment_t *seg;
	uint32_t            idx;

	assert_true(capacity + 1 < ARRAY_MAX);

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 158 at r2 (raw file):

		index++;
	}
	assert_int_equal(index, expected_count);

1. I think the layout you proposed above is the one I like the most.
2. I do not think there is much value in having expected_count defined as a separate variable.

Suggestion:

	int entry_count = 0;
	d_vector_for_each_entry(entry, seg, idx, &vec->dv_list) {
		assert_memory_equal(entry, &state->array[entry_count], vec->dv_entry_size);
		entry_count += 1;
	}
	assert_int_equal(entry_count, capacity + 1);

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 164 at r2 (raw file):

	/** Check if d_vector_free works properly */
	assert_ptr_equal(vec->dv_list.next, &vec->dv_list);
	assert_ptr_equal(vec->dv_list.prev, &vec->dv_list);

I think this test case is complicated enough. I think we should move this bit to double_free_test().

And I think we should use the d_list_*() interface at the lowest. No need to go to its implementation details.

Suggestion:

	/** Check if d_vector_free works properly */
	assert_true(d_list_empty(&state->vec.dv_list));

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 188 at r2 (raw file):

		assert_int_equal(d_vector_size(&state->vec), ARRAY_MAX);
		index++;
	}

As above. You may consider making it a helper.

Suggestion:

	int entry_count = 0;
	d_vector_for_each_entry(entry, seg, idx, &vec->dv_list) {
		assert_memory_equal(entry, &state->array[entry_count], vec->dv_entry_size);
		entry_count += 1;
	}
	assert_int_equal(entry_count, ARRAY_MAX);

[janekmi]

Reviewable status: all files reviewed, 11 unresolved discussions (waiting on @osalyk)

---

a discussion (no related file):
Please make sure your PR passes this check: https://github.com/janekmi/daos/actions/runs/16671433117?pr=29

[osalyk]

Reviewable status: 0 of 2 files reviewed, 11 unresolved discussions (waiting on @janekmi)

---

a discussion (no related file):

Previously, janekmi (Jan Michalski) wrote…

Please make sure your PR passes this check: https://github.com/janekmi/daos/actions/runs/16671433117?pr=29

Done.

---

src/include/daos_srv/d_vector.h line 182 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Got it. The issue is here. Not below.

One more time. Despite the proposed solution was not correct. The credit for finding the failing scenario is all yours. Great job! 🤝

Done.

---

src/include/daos_srv/d_vector.h line 200 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

This bit was actually correct. Please revert it. Thanks!

Done.

---

src/include/daos_srv/d_vector.h line 201 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I rethought this implementation. I like your idea of segment == NULL as the stop condition for the for-loop. But I would like the logic here as organized as possible. I propose to add a little bit of comments to have a common understanding what each case means.

Three key changes:

1. No more load_entry. If segment != NULL there is an entry to be loaded.
2. idx < segment->dvs_len for the current segment. Always. If we run out of slots a new segment has to be loaded.
3. If we run out of segments:

• segment = NULL and
• entry = NULL and
• idx = 0 just to keep it under control. No plan to use it.

Let's discuss.

Done.

---

src/include/daos_srv/d_vector.h line 208 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

We have to discuss this bit offline. I do not understand the purpose of it. 🤔

Done.
it's gone 😉

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 113 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Just to make sure there are at least two segments to free.

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 121 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

1. Spacing:

• operation + check
• newline
• operation + check.

2. d_list_empty -> d_vector_size

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 138 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Definitions first. I prefer the order of appearance.

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 158 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

1. I think the layout you proposed above is the one I like the most.
2. I do not think there is much value in having expected_count defined as a separate variable.

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 164 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I think this test case is complicated enough. I think we should move this bit to double_free_test().

And I think we should use the d_list_*() interface at the lowest. No need to go to its implementation details.

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 188 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

As above. You may consider making it a helper.

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 105 at r2 (raw file):

	assert_int_equal(d_vector_size(&empty), 0);
	assert_true(d_list_empty(&state->vec.dv_list));

Done.

[janekmi]

Reviewed 1 of 2 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @osalyk)

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 132 at r4 (raw file):

	d_vector_free(&state->vec);
	assert_false(d_vector_size(&state->vec));

Please do not implicitly convert integer as Booleans. Sorry. My bad.

Suggestion:

	d_vector_free(&state->vec);
	assert_int_equal(d_vector_size(&state->vec), 0);

	d_vector_free(&state->vec);
	assert_int_equal(d_vector_size(&state->vec), 0);

[osalyk]

Reviewable status: 1 of 2 files reviewed, 2 unresolved discussions (waiting on @janekmi)

---

a discussion (no related file):

Previously, janekmi (Jan Michalski) wrote…

A few more tests I would like to see:

• d_vector_init()
  • think about some tricky entry_size values e.g.
    • entry_size > D_VECTOR_SEGMENT_RAW_CAPACITY
    • D_VECTOR_SEGMENT_RAW_CAPACITY % entry_size != 0
  • d_list_empty(/** ... */) == true
• d_vector_move()
  • move a normally populated vector with an overflow over a single segment.
  • verify the contents of both vectors after the move

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 132 at r4 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Please do not implicitly convert integer as Booleans. Sorry. My bad.

Done.

[janekmi]

Reviewed all commit messages.
Reviewable status: 1 of 2 files reviewed, 10 unresolved discussions (waiting on @osalyk)

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 96 at r5 (raw file):

static void
init_large_entry_size_test(void **state_ptr)

I think this should be the name of this test. If you agree please apply consistently:

• the name of the function
• the name of the test case
• names of the variables in use
• comments.

Suggestion:

init_too_big_entry_size_test

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 102 at r5 (raw file):

	size_t     large_size = D_VECTOR_SEGMENT_RAW_CAPACITY + 1;
	d_vector_init(large_size, &vec_large);
	assert_true(d_list_empty(&vec_large.dv_list));

This init should fail. The vector after its succeeding is no use. You cannot store a single entry.

IMHO we should add this assert to d_vector_init():

D_ASSERT(entry_size <= D_VECTOR_SEGMENT_RAW_CAPACITY);

Good test. 👍

Suggestion:

	expect_assert_failure(d_vector_init(large_size, &vec_large));

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 113 at r5 (raw file):

	d_vector_init(misaligned_size, &vec_misaligned);
	assert_true(d_list_empty(&vec_misaligned.dv_list));
}

I think what you already has done with struct element does the thing.
I might mislead you a bit suggesting otherwise. Sorry. 😉

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 137 at r5 (raw file):

	/** Plan to overload one segmentation */
	int total_entries = (int)(state->vec.dv_segment_capacity * 2 + 1);

It looks like 3 segments in total. Is it intentional?

Code quote:

	/** Plan to overload one segmentation */
	int total_entries = (int)(state->vec.dv_segment_capacity * 2 + 1);

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 152 at r5 (raw file):

	assert_true(d_list_empty(&state->vec.dv_list));

	/** Target should include data */

"include" just sounds odd.

Suggestion:

/** Target should contain data */

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 166 at r5 (raw file):

	d_vector_free(&target);
}

👏

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 175 at r5 (raw file):

	d_vector_init(entry_size, &vec);
	assert_true(d_vector_size(&vec) == 0);
}

It is exactly the same as init_large_entry_size_test().

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 268 at r5 (raw file):

    {"DVEC101: null_vector", append_null_vector_test, setup, teardown},
    {"DVEC102: null_entry", append_null_entry_test, setup, teardown},
    {"DVEC103: large_entry_size", init_large_entry_size_test, setup, teardown},

There is no state in use.

Suggestion:

{"DVEC103: large_entry_size", init_large_entry_size_test, NULL, NULL},

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 269 at r5 (raw file):

    {"DVEC102: null_entry", append_null_entry_test, setup, teardown},
    {"DVEC103: large_entry_size", init_large_entry_size_test, setup, teardown},
    {"DVEC104: misaligned_entry_size", init_misaligned_entry_size_test, setup, teardown},

.

Suggestion:

{"DVEC104: misaligned_entry_size", init_misaligned_entry_size_test, NULL, NULL},

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 282 at r5 (raw file):

{
	const char *test_name = "d_vector_t tests";

You will need this bit for tests expecting assertions to be trigger to succeed.

Code snippet:

d_register_alt_assert(mock_assert);

[janekmi]

Reviewed 1 of 1 files at r5.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @osalyk)

[osalyk]

Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @janekmi)

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 96 at r5 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I think this should be the name of this test. If you agree please apply consistently:

• the name of the function
• the name of the test case
• names of the variables in use
• comments.

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 102 at r5 (raw file):

Previously, janekmi (Jan Michalski) wrote…

This init should fail. The vector after its succeeding is no use. You cannot store a single entry.

IMHO we should add this assert to d_vector_init():

D_ASSERT(entry_size <= D_VECTOR_SEGMENT_RAW_CAPACITY);

Good test. 👍

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 113 at r5 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I think what you already has done with struct element does the thing.
I might mislead you a bit suggesting otherwise. Sorry. 😉

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 137 at r5 (raw file):

Previously, janekmi (Jan Michalski) wrote…

It looks like 3 segments in total. Is it intentional?

Yes, I wanted to force the allocation of more than two segments. Check if d_vector_move() works correctly for a multi-segment vector. Make sure that the data is complete and undamaged.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 152 at r5 (raw file):

Previously, janekmi (Jan Michalski) wrote…

"include" just sounds odd.

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 175 at r5 (raw file):

Previously, janekmi (Jan Michalski) wrote…

It is exactly the same as init_large_entry_size_test().

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 268 at r5 (raw file):

Previously, janekmi (Jan Michalski) wrote…

There is no state in use.

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 269 at r5 (raw file):

Previously, janekmi (Jan Michalski) wrote…

.

Done.

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 282 at r5 (raw file):

Previously, janekmi (Jan Michalski) wrote…

You will need this bit for tests expecting assertions to be trigger to succeed.

Done.

[janekmi]

Reviewed 2 of 2 files at r6, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @osalyk)

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 125 at r6 (raw file):

	d_vector_init(sizeof(struct element), &target);

	/** Plan to overload one segmentation */

Suggestion:

Overload two segment. Three segments in total.

[osalyk]

Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @janekmi)

---

src/utils/dlck/tests/dlck_d_vector_ut.c line 125 at r6 (raw file):

	d_vector_init(sizeof(struct element), &target);

	/** Plan to overload one segmentation */

Done.

[janekmi]

Reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @osalyk)